-
-
Notifications
You must be signed in to change notification settings - Fork 694
feat(component-api-style): add scriptSetupVapor option #2797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: adf9424 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Introduce a new script-setup-vapor option to the Vue component API style rule, allowing <script setup vapor> blocks and enforcing the proper messages when it’s disallowed.
- Extend parser and rule logic to recognize and validate
script-setup-vapor - Add unit tests for both allowing and disallowing
<script setup vapor> - Update documentation with examples and option descriptions
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/lib/rules/component-api-style.js | Added tests for the new script-setup-vapor option and errors |
| lib/rules/component-api-style.js | Parsed, flagged, and reported on <script setup vapor> usage |
| docs/rules/component-api-style.md | Documented the script-setup-vapor option with examples |
Comments suppressed due to low confidence (1)
tests/lib/rules/component-api-style.js:124
- Add a test case for
<script setup>(withoutvaporattribute) when bothscript-setupandscript-setup-vaporoptions are enabled to ensure no error is reported.
options: [['script-setup', 'script-setup-vapor']]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me 🙂
Unfortunately, this new option will not solve the request in #2791 ("detect components using <script setup> and suggest adding the vapor attribute when possible"). Should this be a new rule instead? Or a new additional rule, while first only allowing script-setup-vapor here?
True, that might work. But you have to migrate everything at once to <script setup vapor>, or decrease the rule severity to warn (but then, also other unwanted styles might creep back in).
To implement that, I think we need to check whether it's safe to assign the vapor attribute. I don't know how to do that 🤔
Do you know how to check that?
Uh oh!
There was an error while loading. Please reload this page.
resolve #2791
Waiting for the official release of Vue 3.6...