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

import/extensions should have a ignorePackages option #414

Closed
laurentgoudet opened this issue Jul 4, 2016 · 21 comments
Closed

import/extensions should have a ignorePackages option #414

laurentgoudet opened this issue Jul 4, 2016 · 21 comments

Comments

@laurentgoudet
Copy link

Currently setting import/extensions to always reports the following as an error:

import jQuery from 'jquery';

However, packages default entry point (and extension) should be resolved by the loader, so it doesn't really make sense to do:

import jQuery from 'jquery'/index.js;
@benmosher
Copy link
Member

This makes sense to me.

Feels like another mode between always and never. A flag wouldn't be meaningful in never mode.

@vvo
Copy link

vvo commented Sep 1, 2016

Great! I too want to force extensions only for everything that is not a main entry in package.json

"extensions" : ["error", "always", {allowMainEntry: true}]?

"extensions": ["error", "as-needed"]?

Naming is hard :D

@benmosher
Copy link
Member

benmosher commented Sep 1, 2016

maybe [ "error", "always", "!packages" ]? naming is hard 😅

@vvo
Copy link

vvo commented Sep 1, 2016

Do you have other example of eslint rules with two different string parameters?

I am not comfortable with "!packages", again do you have example of an eslint rule having that sort of string based configuration with a negation?

Why not provide an option object an drop the string based one? Major ftw :D

@benmosher
Copy link
Member

I think a string and then a flag object is probably ideal. No one is surprised by booleans on an options object.

Or maybe ["error", "always", { packages: "never" }], so it's obvious the object is overriding.

I suppose this new behavior would ignore index.js files too. Not sure how to denote that via the flag override name.

@vvo
Copy link

vvo commented Sep 6, 2016

I would not ignore index.js files, only packages.

@benmosher
Copy link
Member

Sure, but if ./foo/index.js exists,

import { x } from './foo' // acceptable
// vs
import { x } from './foo/index' // definitely still reported

I think of index.js being the default value of package.json/main, but maybe that's just me.

@vvo
Copy link

vvo commented Sep 6, 2016

In my project I specifically want to always enforce file extensions on anything that is not the main export of an external package.

Again that's also maybe just me, but this is what I am looking for.

@vvo
Copy link

vvo commented Sep 6, 2016

My goal being: when looking at local imports, I never want to ask myself: "Is this a folder? Is this the index.js file?" => always be specific on local

@benmosher
Copy link
Member

benmosher commented Sep 6, 2016

Ah, got it. I'm not sure that the two are distinguishable, ATM (explicit package main vs. implicit folder index file). (edit: without some additional resolver enhancements)

Would it be workable if it ignored both? or are you depending on this rule to suss out implicit index references?

@vvo
Copy link

vvo commented Sep 6, 2016

Yep for a first start and if it requires a lot of changes it would work for me but ultimately I would love to have something like "disallowImplicit: true" "allowMainPackage: true" :)

@benmosher
Copy link
Member

Yeah, seems like an anti-version of #394. I'd be up for that, but feels like a separate problem (and rule) than this one, just mandating extensions.

@collinsauve
Copy link
Contributor

collinsauve commented May 10, 2017

This really is a must-have in order to use this rule.

(edit: as others have noted, should read "in order to use this rule to require specifying .js on non-module imports")

@ljharb
Copy link
Member

ljharb commented May 10, 2017

(note: it's only a must-have if you want to require extensions for files that are the "main" export of packages - the airbnb config's settings forbid JS extensions, and require them for non-JS files, so it doesn't run into trouble here)

@SimenB
Copy link
Contributor

SimenB commented May 10, 2017

@ljharb then I've misunderstood the issue. My understanding is that the OP doesn't want to use the extension on the main export of packages. Are you saying setting always and having import jQuery from 'jquery'; works now?

@ljharb
Copy link
Member

ljharb commented May 10, 2017

@SimenB no, i'm saying that if you don't choose "always" for JS files, then it works fine.

This issue only exists when you require extensions on .js files.

@SimenB
Copy link
Contributor

SimenB commented May 10, 2017

Ah, gotcha. I've heard import will require file extension according to spec (can't tell where though, might have imagined it!). That might be wrong, though?

Also: https://twitter.com/mathias/status/861843640615829506

@ljharb
Copy link
Member

ljharb commented May 10, 2017

@SimenB in node, import will search for .mjs first. Extensions will continue to be optional, and omitting them will continue to be best practice.

collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 10, 2017
collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 10, 2017
@collinsauve
Copy link
Contributor

Here's a PR: #827

I went with the "ignorePackages" naming, as an option instead of using "always". Open to suggestions for configuration structure if we can reach a consensus.

collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 10, 2017
collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 12, 2017
collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 12, 2017
collinsauve added a commit to collinsauve/eslint-plugin-import that referenced this issue May 12, 2017
@ondasStevo
Copy link

I had the similiar problem and I fixed it with this line of code in my .eslintrc config:
"import/extensions": ["error", "never", { "packages": "always" }]

And in component I call resources like this:
import React from 'react';
import Sidebar from '../../sidebar/Sidebar';

@kruegelstein
Copy link

We have the same problem. We want to always require the "js" extension because we're also having "jsx" extensions and want to avoid confusion. Unfortunately there is still no option which fixes this for packages. The comment above is about never requiring extensions so it won't fix anything :/

millerized pushed a commit to millerized/eslint-plugin-import that referenced this issue Oct 30, 2017
ljharb pushed a commit to collinsauve/eslint-plugin-import that referenced this issue Oct 31, 2017
@ljharb ljharb closed this as completed in 8def087 Nov 2, 2017
ljharb added a commit that referenced this issue Nov 2, 2017
[new] import/extensions should have a ignorePackages option.

Fixes #414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants