Skip to content

Fix: Add registeredComponentsOnly option to component-name-in-template-casing rule #714

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

Merged
merged 7 commits into from
Jan 3, 2019

Conversation

ota-meshi
Copy link
Member

@ota-meshi ota-meshi commented Dec 6, 2018

This PR makes the following changes to component-name-in-template-casing rule.

  • Add registeredComponentsOnly option.
  • Change the default for registeredComponentsOnly options to true.
  • Add globalRegisteredComponents option.
  • Add globalRegisteredComponentPatterns option.
  • Changed to be able to set regexp for ignores option.

The feature of each additional option is as follows.

  • registeredComponentsOnly ... If true, only registered components (in PascalCase) are checked. If false, check all.
    default true
  • globalRegisteredComponents (string[]) ... (Only available when registeredComponentsOnly is true) The name of globally registered components.
  • globalRegisteredComponentPatterns (string[]) ... (Only available when registeredComponentsOnly is true) The pattern of the names of globally registered components.

https://vuejs.org/v2/guide/components-registration.html#Name-Casing

Since the casing of the globally registered component is unknown, we made an option to check only the local registered components that are explicitly used.
Also, I changed this option to work by default.

This fixes the problem pointed out below.

Fixed #710 #705

@fxxkscript
Copy link

fxxkscript commented Dec 21, 2018

Now, if i use this option in single file component, it will change component name in template but not in js. Will this PR fix this issue?

<template>
<ComponentA> // this changes
</template>
<script>
var ComponentB = {
  components: {
    // this is a bug, it did not change this key
    'componentA': ComponentA
  },
}
<script>

#736

@ota-meshi
Copy link
Member Author

@fxxkscript Hi.

If you have the following .vue file and you do not want to convert the casing of the elements in the template, I think that issue will be solved.

<template>
<componentA>
</template>
<script>
// @vue/component
var ComponentB = {
  components: {
    'componentA': ComponentA
  },
}
</script>

@michalsnik
Copy link
Member

Good job @ota-meshi I like the idea with registeredComponentsOnly option.

I also have a question though, what do you think about simplifying the API a little bit? That is, having only these two:

  • ignores
  • registeredComponentsOnly

Isn't globalRegisteredComponents and ignores kinda the same? If we registered something globally and want to ignore it, we could just put it there as till now. With one update though - we could allow using regex inside, to support Vuetify case etc.

Then registeredComponentsOnly would work exactly as you wrote - ignoring all components, but those explicitly registered. It would however still support ignores array :)

@ota-meshi
Copy link
Member Author

@michalsnik I think that your proposal is simple and good!

I will remove the globalRegisteredComponents option and allow the regexp in ignores option.

@ota-meshi
Copy link
Member Author

I updated it. Please check again.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome @ota-meshi Good job :) I left only two suggestions.

const allowedCaseOptions = ['PascalCase', 'kebab-case']
const defaultCase = 'PascalCase'

const RE_REGEXP_CHAR = /[\\^$.*+?()[\]{}|]/gu
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding comment above for a reference with example string that it describes? Or even better, write unit test for each REGEX? From my experience unit tests serve the best documentation for regular expressions :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalsnik
Thank you for review! I added test cases.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done @ota-meshi

@michalsnik michalsnik merged commit b48dc16 into master Jan 3, 2019
@michalsnik michalsnik deleted the fix-component-name-in-template-casing branch January 3, 2019 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vue/component-name-in-template-casing transforms <b-button> to <BButton>.
4 participants