-
-
Notifications
You must be signed in to change notification settings - Fork 696
⭐️New: Add rule require-component-name
#589
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
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.
@hirokiosame Can you add test for its going to prompt about missing name
export default { name }
new Vue({})
Those one are failing
Vue.component('my-component', { })
Vue.mixin({ })
#265 Additionally we should think what should happens with mixins because for now we have
const componentComments = sourceCode.getAllComments().filter(comment => /@vue\/component/g.test(comment.value))
we should consider adding @vue/mixin? we have no way of telling if something is mixin or component same for Vue.mixin()
Hey, have you looked at the discussion here #265 @hirokiosame ? The PR @chrisvfritz mentioned has been merged and we didn't yet agree whether we want to introduce this rule or not. I'll put on-hold for now and let's continue discussion there, as I have very mixed feelings about it.
require-component-name (追記ここまで)
Ah great thanks guys. Let me know what you guys decide on and I can work on it further.
Can we add an option force the component name to match the file name?
e.g.
This is correct::
// src/components/MyComponent.vue <script> export default { name: 'MyComponent', } </script> <template />
This is incorrect::
// src/components/MyComponent.vue <script> export default { name: 'MyCompnent', // missed the o } </script> <template />
It would help to catch cases where someone mistyped the component name.
@rodrigopedra I'd be open to a rule for this, though I think it would mostly only be useful in .jsx files. If you're using .vue files, the only time you should ever need to specify a name for the component should be when the component is recursive.
Hi @chrisvfritz , thanks for the reply. If possible I wanted to understand the rationale for not specifying a name property for any component. I always set them for consistency but perhaps it is a bit of over engineering on my side.
Anyway, I adapted this PR code to compare the filename to the component's name property, as it was my first time writing a eslint plugin I don't know if it is written the best way.
/** * @fileoverview Require components to have names * @author Hiroki Osame <hiroki.osame@gmail.com> */ 'use strict'; // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ const utils = require( 'eslint-plugin-vue/lib/utils' ); module.exports = { meta : { docs : { description : 'require components to have names', category : 'recommended', url : 'https://github.com/vuejs/eslint-plugin-vue/blob/v5.0.0-beta.3/docs/rules/require-component-name.md', }, fixable : null, schema : [], }, create ( context ) { return utils.executeOnVueComponent( context, ( object ) => { const nameProperty = object.properties.find( ( prop ) => prop.key.name === 'name' ); const filename = context.getFilename().replace( /^.*?([a-z0-9_-]+)\.(?:vue|jsx)$/i, '1ドル' ); if ( !nameProperty ) { context.report( { obj : object, loc : object.loc, message : `Expected component to have a name (${filename}).`, } ); return; } const name = nameProperty.value.value; if ( name !== filename ) { context.report( { obj : object, loc : object.loc, message : `Component name should match filename (${filename} / ${name}).`, } ); } } ); }, };
For using it only on jsx files I can change the regular expression that extracts the filename and apply only for jsx files.
If you can, please review it and I can send a PR or ask @hirokiosame to add it to this PR.
Thanks!
If possible I wanted to understand the rationale for not specifying a name property for any component. I always set them for consistency but perhaps it is a bit of over engineering on my side.
@rodrigopedra Great question! 🙂 My reasoning is actually the same as yours: consistency. 😄 When we always use .vue files and don't specify a name, we guarantee that the component's name always matches the file name. When you specify the name again in a name property, you're not only writing more code unnecessarily, but also forcing yourself to either manually make sure the two names match (to avoid confusing behavior) or write some kind of tooling to help you avoid mistakes, like you're doing now. 🙂
If you can, please review it and I can send a PR
Sounds great. This would be a new rule (maybe match-component-file-name), so a new PR would be best. Writing unit tests for the rule will also help us review it, because they'll not only describe the behavior you're trying to create, but if the tests are robust and they pass, we can focus on higher level aspects of the code.
@chrisvfritz thank you very much for the feedback, I will try to send a PR later this week.
Hi @hirokiosame , check the work that was done #668 , it was initially based in your code in this PR, and we defaulted to jsx and added options to other file extensions.
Since the #668 have been finally merged to master. Can we close this PR, or is there something we want to do extra here @chrisvfritz ?
@michalsnik I think this can be closed unless someone objects.
This rule requires components be named