-
-
Notifications
You must be signed in to change notification settings - Fork 681
⭐️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 |
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 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 <[email protected]>
*/
'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 If you can, please review it and I can send a PR or ask @hirokiosame to add it to this PR. Thanks! |
@rodrigopedra Great question! 🙂 My reasoning is actually the same as yours: consistency. 😄 When we always use
Sounds great. This would be a new rule (maybe |
@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 |
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