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

Change some options for prevent-abbreviations rule #422

Merged

Conversation

fisker
Copy link
Collaborator

@fisker fisker commented Oct 18, 2019

The PR will make this code bellow report an error by default

const getDocsUrl = require('./utils/get-docs-url');

without breaking #237 (comment)

import tempWrite from 'temp-write';

and

import tempWrite from './node_modules/temp-write';

moduleId starts with . / @/ and without node_modules is considered internal-module.

The internal naming and internal-module logic may need discuss.

//cc @futpib @sindresorhus

@futpib
Copy link
Contributor

futpib commented Oct 18, 2019

What is the meaning of a module name starting with @/?

@fisker
Copy link
Collaborator Author

fisker commented Oct 18, 2019

@ is webpack alias, means project root

@fisker
Copy link
Collaborator Author

fisker commented Oct 18, 2019

reminds me there is also a ~, or maybe we do external check, any thoughts?

@futpib
Copy link
Contributor

futpib commented Oct 18, 2019

Webpack aliases can be absolutely arbitrary and @ as an alias for the root of the project is not something I've seen used. I don't think we should support something that is not even a widespread convention (and definitely not a standard).

Anyway, what is the issue you are trying to solve with this PR?

@fisker
Copy link
Collaborator Author

fisker commented Oct 18, 2019

I want imports report error if name is using abbreviation, but #237 (comment) don't want check npm packages import, so I made this PR to only target internal-modules resolve things like example I wrote in the comment above , and this

import pkg from '../package.json';
It's not reported because it's import .

@fisker
Copy link
Collaborator Author

fisker commented Oct 22, 2019

@futpib @sindresorhus Are you ok with this change?

@sindresorhus
Copy link
Owner

Webpack aliases can be absolutely arbitrary and @ as an alias for the root of the project is not something I've seen used. I don't think we should support something that is not even a widespread convention (and definitely not a standard).

👍

@sindresorhus
Copy link
Owner

Are you ok with this change?

Yes

@sindresorhus sindresorhus changed the title Change checkDefaultAndNamespaceImports & checkShorthandImports default value to internal Change some options for prevent-abbreviations rule Nov 12, 2019
@sindresorhus sindresorhus merged commit 4d5f5cb into sindresorhus:master Nov 12, 2019
@fisker fisker deleted the prevent-abbreviations-default-options branch November 12, 2019 12:03
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 this pull request may close these issues.

3 participants