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/no-unresolved should find json files #333

Closed
jfmengels opened this issue May 11, 2016 · 12 comments
Closed

import/no-unresolved should find json files #333

jfmengels opened this issue May 11, 2016 · 12 comments
Assignees
Labels
Milestone

Comments

@jfmengels
Copy link
Collaborator

jfmengels commented May 11, 2016

I'm trying to add the plugin to a repo I'm working on, and I'm getting false positives with no-unresolved when importing JSON files.

import setEvent from './fixtures/foo'; // <-- `./fixtures/foo.json` is an existing and valid JSON file

I'm not aware of it being incorrect to import a JSON file, but please let me know if that's the case.

@benmosher
Copy link
Member

Weird. Assuming resolve supports it, I think it should. Sounds like a bug.

@benmosher benmosher added the bug label May 15, 2016
@benmosher benmosher added this to the next milestone May 15, 2016
@benmosher benmosher self-assigned this May 15, 2016
@shark0der
Copy link

shark0der commented May 26, 2016

Later edit: I had a similar issue and fixed it myself. The culprit was the node_modules scanner: http://stackoverflow.com/a/31656014/539153 (I edited the answer meanwhile).

Somehow, I still don't understand what exactly is going on, but when eslint is invoked with a different cwd (other than project's root), node_modules obviously isn't pointing to what was originally intended and the resolver fails. Fixed it by passing path.resolve(__dirname, 'node_modules'); to fs.readdirSync

@benmosher
Copy link
Member

benmosher commented Jun 3, 2016

@jfmengels I just added tests but they are passing. Can you take a look? If you can construct a failing test, I can patch it, but I'm not sure what the issue is. Maybe something with your config?

@benmosher benmosher removed this from the next milestone Jun 3, 2016
@jfmengels
Copy link
Collaborator Author

Hmm... Yeah, I'll try to take a look at it tonight.

@benmosher
Copy link
Member

No rush, speed is up to you. AFAICT it's working. (also, sorry it took so long for me to look at this!)

jfmengels added a commit to jfmengels/eslint-plugin-import that referenced this issue Jun 6, 2016
jfmengels added a commit to jfmengels/eslint-plugin-import that referenced this issue Jun 6, 2016
@jfmengels
Copy link
Collaborator Author

No worries, in fact, I had actually forgotten I had created this issue...

Ok, so the problem is not when you try to do
import foo from "./bar.json"
but when you try to do
import foo from "./bar" (where ./bar is actually ./bar.json).

Adding .json at the end actually solves the problem, but then it showed additionnal errors in default and namespace

image

I've created test cases in my "fork" of issue-333 here (don't have write rights to add to the branch). I was able to create failing test cases for all three rules. Let me know if you need help with this, and thanks for what you did up till now :)

@benmosher
Copy link
Member

benmosher commented Jun 6, 2016

Ah, yeah, so for this, import/extensions is the correct solution ATM: https://github.com/benmosher/eslint-plugin-import#importextensions

I think that should resolve it. See note in README about making ['.js'] the default in v2.

@benmosher
Copy link
Member

Also, I'm confused why you don't have push rights to a branch, I thought you could push anywhere in the repo?

@jfmengels
Copy link
Collaborator Author

Also, I'm confused why you don't have push rights to a branch, I thought you could push anywhere in the repo?

Fixed it, my bad. Didn't configure my git remote stuff well. Just pushed my issue-333 branch over yours now.

And the import/extensions tip works 👍

@benmosher
Copy link
Member

Ah, wild. This is actually an "issue" with node-resolve. By default it only finds .js files.

I just need to add .json to the Node resolver and this should work.

@jfmengels
Copy link
Collaborator Author

😄

@benmosher
Copy link
Member

Just published with node resolver v0.2.1

@jfmengels jfmengels mentioned this issue Jun 30, 2016
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants