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

New rule: no-reserved-imports #87

Open
michalsnik opened this issue Jul 6, 2017 · 9 comments
Open

New rule: no-reserved-imports #87

michalsnik opened this issue Jul 6, 2017 · 9 comments
Labels
Enhancement New Rule Idea for a new lint rule

Comments

@michalsnik
Copy link
Member

According to https://github.com/ember-cli/ember-rfc176-data I think we can add a rule that will check if reserved identifiers like Object are being imported.

Incorrect:

import Object from '@ember/object';

Correct:

import EmberObject from '@ember/object';

All reserved keys are here: https://github.com/ember-cli/ember-rfc176-data/blob/master/reserved.json

@michalsnik
Copy link
Member Author

cc @Turbo87 ☝️

@t-sauer
Copy link
Contributor

t-sauer commented Jul 27, 2017

You can already do this with ESLint with this rule:

'no-shadow': ['error', { 'builtinGlobals': true }]

@michalsnik
Copy link
Member Author

Oh, true @t-sauer. Then I think we should add this to recommended config.

@Turbo87
Copy link
Member

Turbo87 commented Aug 1, 2017

Then I think we should add this to recommended config.

that would require a breaking change release again and I'm very much against that in the foreseeable future. IMHO we should only do breaking change releases if we actually need to break how a rule behaves, the recommended config should ideally live outside of this repo/project.

@Turbo87
Copy link
Member

Turbo87 commented Aug 1, 2017

@t-sauer the potential advantage of having a custom rule would be that it can be --fix-able, but it's probably not worth it in this case 🤔

@kevinkucharczyk
Copy link
Contributor

I found that there was a similar suggestion made to the Airbnb eslint base and it was not merged, with this as the reasoning: airbnb/javascript#667 (comment)
So similarly, I don't think we should be including the builtinGlobals option in our configuration.

@jbandura
Copy link
Collaborator

jbandura commented Aug 8, 2017

I think that having a dedicated fixable rule just for the ember object, string, etc. might be a good idea, I'd personally find it valuable.

@michalsnik
Copy link
Member Author

Ok, so it looks like indeed having a dedicated rule will be most valuable in this case.

@jbandura
Copy link
Collaborator

Picking this up then - will probably make a PR around saturday/sunday

@bmish bmish added the New Rule Idea for a new lint rule label May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New Rule Idea for a new lint rule
Projects
None yet
Development

No branches or pull requests

6 participants