Re-enable vue/multi-word-component-names

The following discussion from !84623 (merged) should be addressed:

• @markrian started a discussion: (+2 comments)

Actually, looking at app/assets/javascripts/content_editor/components/bubble_menus/formatting.vue has made me realise that the way the lint rule is written doesn't make sense It doesn't matter what the component's internal name is, only the name under which it's registered.

That is, the rule does not complain about this hypothetical step_component.vue file:

<script>
export default {
  name: 'StepComponent',
  // etc...
};
</script>

So far so good. But then where it's used:

<script>
import step from './step_component.vue';

export default {
  components: {
    step,
  },
};
</script>

<template>
  <step>foo</step>
</template>

This is surely incorrect, since if tomorrow a native <step> element is defined, this will break/behave unexpectedly (once vue is updated anyway).

This definitely seems like an oversight, since it does prevent something like Vue.component('step', {}).

So, I think I'm going to have to backtrack on this Sorry @afontaine and everyone else! I think we should:

1. Disable the rule in this MR
2. Open an MR to disable it in @gitlab/eslint-plugin
3. Open an issue upstream for this
4. Open follow-up issues to re-enable the rule once this is address upstream.

For now, this issue represents all next steps. This issue should be broken down into separate issues.

mentioned in commit gitlab-renovate-forks/gitlab@84e21424

added frontend groupfoundations maintenanceworkflow labels

added typemaintenance label

changed milestone to %Backlog

Setting label(s) devopsecosystem sectiondev based on groupfoundations.

added sectiondev + 1 deleted label

added [deprecated] Accepting merge requests label

added devopsmanage label and removed 1 deleted label

mentioned in merge request !116464 (merged)

mentioned in merge request !124749 (closed)

@markrian wouldn't enabling this rule be incrementally better than not having it? I think it would encourage better naming in any case.

@mrincon Probably! I think using any of the official Vue linting rules is a pretty safe call. If you're interested in tackling this, please do!

!126678 (closed), we have 213 occurrences, and some pretty scary ones like form and table

@markrian this rule has been enabled (override removed from .eslintrc).

I am ready to close this from my part, wdyt?

I think it's worth updating the docs for the name: alternative, as mentioned in !127929 (comment 1497047404). I think requiring renames of files and imports is too onerous, and is busy-work. Although, you said:

I think the alternative with name leaves us open the same problems this is trying to address.

So perhaps there's something I'm missing. Can you explain? Given the lint rule allows single-word file names with a multi-word name: fields, why shouldn't we allow it?

Can you explain?

The case of having <table> in our templates represent something other than the HTML <table>. I think having an alternative name: (e.g. UsersTable) does not address this. (right?)

Certainly contributors could "misuse" the imports to go back and end up with <table> in their template but that's not the norm, and it is easy to catch during reviews.

requiring renames

During the reviews of the MRs to enable the rules I had to rebase to account for new occurrences of single-word components, with the rule enabled at least we prevent folks from adding more.

I don't have plans to require a rename, I think they would simply happen over time as during the development and refactors. wdyt?

The case with <table> you describe could happen either with a name: or a multi-word file name, since SFCs have a default (unnamed) export. For instance:

import table from './user_table.vue';

export default {
  components: {
    table,
  }
}

Having said that, I see different lint errors coming up when I try that (see bottom of screenshot: vue/no-unused-components: The "table" component has been registered but not used.):

To fix that, I have to use <Table> in the template, but then a new error appears (vue/no-reserved-component-names: Name "table" is reserved in HTML.):

In other words, I don't think the vue/multi-word-component-names lint rule affects this issue.

This goes back to my original comment:

It doesn't matter what the component's internal name is, only the name under which it's registered.

This is sort of corroborated by the docs about the name: property:

Note that when a component is registered globally with Vue.component(), the global ID is automatically set as its name.

I even suspect that the name property is derived by the SFC compiler from the file name, if it doesn't already have a name. I mean, it must be, right? Why would there be a second way of annotating a component's name at runtime? Still, it'd be good to check this with the compiler itself.

Anyway, that's enough rambling. What's my point? I think my point is, I'm still not sure vue/multi-word-component-names is of any use to us, since we exclusively use compiled SFCs (not in-DOM templates), and the cases we actually want to avoid (the <table> above) are prevented by other lint rules.

Given I don't think it's of much use to us, I would prefer we document the least invasive "fix" for warnings from this rule, which is to set the name property, rather than rename the file.

Why would there be a second way of annotating a component's name at runtime?

Hah, well, it turns out there aren't two... but four!

@markrian, the current guidelines I left at https://docs.gitlab.com/ee/development/fe_guide/tooling.html#vuemulti-word-component-names-is-disabled-in-my-file would encourage users to rename their file. Do you want to change this recommendation? I'd happily merge an MR with such a change.

@nicolewilliams closed this issue and I that pushed me to reply here even though I kind of want to leave this as-is , because I still feel preventing single-word names helps our contributors to do the right thing by default and I am content with the current recommendation.

mentioned in merge request !126678 (closed)

@nicolewilliams I'd like to tackle this during %16.3, it can cause collisions between our HTML and Vue namespaces, how would we go about adding this to our planning?

changed milestone to %16.3

assigned to @mrincon

added workflowin review label

mentioned in merge request !127929 (merged)

mentioned in merge request !127930 (merged)

closed