-
Notifications
You must be signed in to change notification settings - Fork 95
refactor: deprecate events not comply with Vue event naming rules #7058
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
|
/backport to stable8 |
ff0594d to
8f3f370
Compare
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.
Nice of you to keep the deprecated ones for a while 👍
CHANGELOG.md
Outdated
| It is replaced by `arrowEnd` which reflects that the directions depends on the text directions (LTR vs RTL). | ||
|
|
||
| #### Event names | ||
| With Vue 3 it is recommended to use camel case event names and only scope events for models like `update:modelValue` or `update:shown`. |
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.
Same as: #7059 (comment)
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.
You might be right and I misinterpreted the docs.
But still we only have this two places, resize:list also makes not much sense it should rather be consistent with all other events we have (resize-list).
For interact:todo it might make sense if there are any other events for interaction, but its the only one so not sure.
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.
You might be right and I misinterpreted the docs.
This might also be from the eslint-plugin-vue member's comments, where foo:barBaz is not considered camelCase, and : is considered a special case for update:* events (that are "never used without v-model/.sync directive").
IMO, this is only a personal opinion, and Vue documentation never says it directly.
I saw namespaced event names in many apps and good libs, which is a great thing for complex components.
There was added an additional option for ESLint rule for this case:
vuejs/eslint-plugin-vue#1321
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.
But still we only have this two places,
resize:listalso makes not much sense it should rather be consistent with all other events we have (resize-list).
Agree as resize: is not a namespace.
For
interact:todoit might make sense if there are any other events for interaction, but its the only one so not sure.
Yes, but future-proof.
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.
So I'm fine with renaming in general.
But disagree with "With Vue 3 it is recommended ... only scope events for models" part
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.
For interact:todo it might make sense if there are any other events for interaction
I am fine with renaming it bc now it conveys it as big feature whereas it's just one simple interaction and we don't have any plans to expand interaction.
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.
Agree! Adjusted the changelog.
About the scoping: I do not have a problem with interact:todo but it seems its the only one where this is needed so we could also just use the plain event. But if you like then I revert that @ShGKme and add it as an exception to the eslint config.
…e-list` Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
8f3f370 to
bf86f78
Compare
☑️ Resolves
Deprecate events which not comply with camelCase naming.
Only those two places have an invalid naming as normally only the
update:MODELevent is scoped and other events just use camel case names likeselectSomething(akaselect-something) instead ofselect:something.🏁 Checklist
stable8for maintained Vue 2 version or not applicable