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

packageDir should be relative to eslint config file not cwd #1061

Closed
stuartf opened this issue Apr 3, 2018 · 11 comments
Closed

packageDir should be relative to eslint config file not cwd #1061

stuartf opened this issue Apr 3, 2018 · 11 comments

Comments

@stuartf
Copy link

stuartf commented Apr 3, 2018

I'm using https://github.com/w0rp/ale to run eslint inside vim but packageDir seems to be resolved relative to where I start vim instead of where the eslint config file is. So if I have the rule 'import/no-extraneous-dependencies': ['error', { packageDir: '.' }] and do vim src/index.js everything works as expected, but if I do cd src; vim index.js I get import/no-extraneous-dependencies: The package.json file could not be found.

@ljharb
Copy link
Member

ljharb commented Apr 10, 2018

That seems like an issue with "ale"; you'll always want to set your CWD as the package root no matter where you opened vim from. It might be a bug/feature request to file with them?

Do you get the same issue when running eslint on the command line? If not, it's not an issue with this plugin.

@oblador
Copy link

oblador commented Apr 10, 2018

I have a similar issue with SublimeLinter.

@ljharb
Copy link
Member

ljharb commented Apr 10, 2018

@oblador how are you opening your project in sublime?

@stuartf
Copy link
Author

stuartf commented Apr 10, 2018

Yes, the problem occurs on the commandline as well:

──┤stuart@varan:pts/11├───────────┤/home/stuart/src/gatechLTIStarter├┤master├──
──┤1051:Tue,10 Apr 18:☠├── ./node_modules/.bin/eslint -c ./.eslintrc.js src/components/App.js
──┤stuart@varan:pts/11├───────────┤/home/stuart/src/gatechLTIStarter├┤master├──
──┤1051:Tue,10 Apr 18:✓├── echo $?
0
──┤stuart@varan:pts/11├───────────┤/home/stuart/src/gatechLTIStarter├┤master├──
──┤1051:Tue,10 Apr 18:✓├── cd src/
──┤stuart@varan:pts/11├───────┤/home/stuart/src/gatechLTIStarter/src├┤master├──
──┤1051:Tue,10 Apr 18:✓├── ../node_modules/.bin/eslint -c ../.eslintrc.js components/App.js 

/home/stuart/src/gatechLTIStarter/src/components/App.js
  0:1  error  The package.json file could not be found  import/no-extraneous-dependencies

✖ 1 problem (1 error, 0 warnings)

──┤stuart@varan:pts/11├───────┤/home/stuart/src/gatechLTIStarter/src├┤master├──
──┤1051:Tue,10 Apr 18:☠├── 

@ljharb
Copy link
Member

ljharb commented Apr 10, 2018

Thanks, that clarifies things a bit. Possibly related to #458 but this sounds different.

@w0rp
Copy link

w0rp commented Apr 30, 2018

One thing that could be considered would be adding an extra option for making the packageDir setting relative to the configuration file. That way you wouldn't break existing configurations, and a configuration like this could work. Or it could accept an object with the flag instead of a string, etc.

@benmosher
Copy link
Member

sounds like much better behavior. I think this would qualify as semver-major breaking change though, FWIW.

IIRC one workaround is to define your ESLint config as JS and use __dirname as part of a path.join.

@benmosher
Copy link
Member

benmosher commented Oct 10, 2018

can anyone confirm how to read the eslint config file's location? last I knew, this is not info that is accessible in the plugin.

if it is not possible, will have to close with the resolution to use the workaround I stated above.

also, I just read @w0rp's suggestion to use a flag to route relative to config file. If it's possible to do, will consider as a short-term semver-minor change but I think I'd still want to make it the default behavior in the next major version.

@ljharb
Copy link
Member

ljharb commented Oct 10, 2018

I don’t believe there is; config can come from multiple locations, including inline comments, and be merged upwards as well - so there may not be any config file locations, or there may be more than one.

@w0rp
Copy link

w0rp commented Oct 10, 2018

I think using __dirname is an effective solution with a JS configuration file. I've done similar things for other configuration files before.

@benmosher
Copy link
Member

closed by adding a note to docs. thanks all!

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

No branches or pull requests

5 participants