-
-
Notifications
You must be signed in to change notification settings - Fork 681
Rule proposal: require-component-name
#265
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
Comments
Hey, thanks for posting this proposal. I think this might even be something worth to be mentioned in Styleguide itself / cc @chrisvfritz |
Hmm, I would probably leave this out of the style guide and keep it in the uncategorized section here, because a
So now that I think about it actually, and especially since functional components can now be written in |
The name is necessary when rendering a component recursively. |
Yes, that's true! I think a rule to warn about recursive components without a name would be useful. I'd put that in strongly-recommended, but probably leave it out of the style guide still because it's a bit niche. |
My use case is slightly different. This is a extremely simplified version of one my files: import Table from '../components/Table'
export default {
name: 'SpList',
props: {
visits: { type: Array, default: [] }
},
render() {
let { visits } = this
return (
<div>
<Table
header={['Location', 'Date', 'Users', 'Status']}
items={visits} factory={rowFactory}
/>
</div>
)
}
} Notice how I never registered While I have to say that I probably said "my use case" too many times to make this proposal global to the Vue community, I think it is importante enough to people using babel-plugin-transform-vue-jsx (which is probably a lot of people!) |
@danielbastos11 The problem isn't actually that that you're using JSX, but rather that you're not using a <script>
import Table from '../components/Table'
export default {
name: 'SpList',
props: {
visits: { type: Array, default: [] }
},
render() {
let { visits, $style } = this
return (
<div class={$style.tableContainer}>
<Table
header={['Location', 'Date', 'Users', 'Status']}
items={visits} factory={rowFactory}
/>
</div>
)
}
}
</script>
<style lang="scss" module>
.tableContainer {
/* ... */
}
</style> I guess maybe a rule that's missing from the style guide is to use |
@chrisvfritz I agree that using .vue files is preferable, but consider this scenario: If I use a component someone published, a file that was bundled with rollup or webpck - that component will not have a And if I use it like @danielbastos11 does, I will end up with an anonymous component in the devtools - so I'm forced to register it locally just to get a name or manually add a name to it. The same woudl happen if I passed such dynamically to ...or am I missing something? |
@LinusBorg If This also makes me wonder if there should be a separate rules category (and maybe section of the style guide or completely separate page) for 3rd-party component libraries. |
@chrisvfritz Thanks a lot for your feedback! I don't think a rule recommending the use of |
BTW, I'd agree that adding names to a library's component should be the library's author concern. If the 3rd-party components I'm using aren't named, I should either make a PR or live with it. |
@danielbastos11 I might be misunderstanding, but how would the recommendation for Also, "plain old JSX files" are already a special kind of file that requires preprocessing. |
@chrisvfritz I meant that by not making I know these people could just turn the rule off on their .eslintrc, but I've seen many great component libraries just assume everybody is using |
Would everybody be ok with enforcing naming components created on |
I'm still not convinced by this, as the whole point of a style guide is to give people rules to follow that will unequivocally make their life easier. Choosing not to use I agree that a If I could be provided with a potential downside, I'd be open to the rule, but otherwise I fear it would imply a recommendation for a pattern I don't see a valid use case for. |
I guess I just see Single File Components and JSX as two completely independent features, and I don't see the point in tying them up together like that. Anyway, I think this rule will be a valuable contribution to the community even if limited to |
That sounds good to me. 🙂 I agree it should be in |
Currently we don't detect non-SFC, we detect all Vue components, both SFC and nonSFC. Theoretically we could check if the file has |
Another potential use case for this rule is that vue-test-utils detects component presense by name IIRC. Also fwiw, I don't think we gain anything by making the plugin only enforce style guide rules? This could easily sit under an "extras" rule for people who want to use it; something like "These rules are not part of the style guide, but my be optionally enforced in your project if you choose". It seems a little excessive to have an entirely separate plugin for that. |
@robcresswell We actually have an "Uncategorized" section for rules not in the style guide. 🙂 |
@robcresswell vue-test-utils defaults to use the constructor to identify component instances, so there's no requirement for the component to have a name |
@eddyerburgh Ah, I'm unsure where I had that bit of info stored from. Thanks for clarifying. |
@chrisvfritz So you do! Well, anyway, I'll leave it as "I'm rather a fan of this rule". |
Just following up with an update. Once this vue-loader PR is merged, a So then I see two possible uncategorized rules that are not mutually exclusive:
What are thoughts on these? |
@chrisvfritz I personally don't like this feature to be added in |
@sodatea I agree a vue-loader option to prevent adding In the cases you provided though, a user can provide a As a side note though, I discourage people from using the component organization strategies you provided as examples. The reason is the vast majority of navigation in most projects tends to use fast file switching in editors, which this strategy works very poorly with. In those dropdowns, the filename is typically front and center, while the path is grayed out and takes longer to see. With all generic components in on flat folder, it's also takes only a second or two to bring up lists of related components, including their hierarchy. Does that make sense? |
The |
Please describe what the rule should do:
This rule warns you if the component's
name
property is undefinedWhat category of rule is this?
[x] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)
Provide 2-3 code examples that this rule will warn about:
Why should this rule be included
Naming components is a good practice. It makes it easier to debug your code with Vue Devtools, and helps you to keep your code organized. So much so that at least 2 rules on the Style Guide address the issue of how to name your components - one of them is a essential rule.
The text was updated successfully, but these errors were encountered: