Skip to content
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

prevent-abbreviations default option is not good #271

Closed
fisker opened this issue Mar 28, 2019 · 10 comments · Fixed by #404
Closed

prevent-abbreviations default option is not good #271

fisker opened this issue Mar 28, 2019 · 10 comments · Fixed by #404

Comments

@fisker
Copy link
Collaborator

fisker commented Mar 28, 2019

prevent-abbreviations is really good, I have tried to lint my old scripts.

after a few test, I notice that

checkDefaultAndNamespaceImports default value is false, but normally change the name of default and namespace import has no side effect, this should set to true, I think.

checkProperties default value is true, but normally when export or return a object, the prop name can't just rename without api changes, I think this should set to false by default.

@fisker
Copy link
Collaborator Author

fisker commented Mar 28, 2019

This is what I'm using, feel better when linting, no need change api (including internal using props)

    'unicorn/prevent-abbreviations': [
      'error',
      {
        checkDefaultAndNamespaceImports: true,
        checkProperties: false,
      },
    ],

@futpib
Copy link
Collaborator

futpib commented Mar 28, 2019

Could you please split this into two separate issues (one for each default we've chosen)?

@futpib
Copy link
Collaborator

futpib commented Mar 28, 2019

Some reasons behind checkDefaultAndNamespaceImports: true are outlined in the discussion starting from here #237 (comment)

Regarding checkProperties, we currently ignore names exported via export keyword. Maybe we should ignore module.exports too.

@fisker
Copy link
Collaborator Author

fisker commented Mar 28, 2019

thanks for explaining checkDefaultAndNamespaceImports.

for checkProperties, I don't think export / module.exports is good enough, how do you know return is not a kind of "exporting", maybe just for another function to use, or handwriting umd or iife mod.

since those two options can easily overwrite, I'm going to keep my own choices.

@sobolevn
Copy link

sobolevn commented Apr 1, 2019

I am facing this issue too. Since the new release linter checks options that I do not have control over. That's sad.

Failing build: https://travis-ci.org/wemake-services/wemake-vue-template/builds/513997750
Code: https://github.com/wemake-services/wemake-vue-template/blob/master/template/nuxt.config.ts#L93

This is a default Nuxt function which works with the default API:

{
  'build': {
      // Error here on `isDev`:
      extend (config, { isDev, isClient }) { ... }
  }
}

@fisker
Copy link
Collaborator Author

fisker commented Aug 21, 2019

@sindresorhus
Copy link
Owner

Regarding checkProperties, we currently ignore names exported via export keyword. Maybe we should ignore module.exports too.

👍

@sindresorhus
Copy link
Owner

for checkProperties, I don't think export / module.exports is good enough, how do you know return is not a kind of "exporting", maybe just for another function to use, or handwriting umd or iife mod.

@fisker Can you provide some code examples of this?

@sindresorhus
Copy link
Owner

I think we should make checkProperties false by default for now.

@sindresorhus
Copy link
Owner

@sobolevn That's weird. According to our docs, checkShorthandProperties is false by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants