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

Webpack resolver looks for custom webpack config in packages with jsnext:main #666

Open
tf opened this issue Nov 14, 2016 · 20 comments · May be fixed by #994
Open

Webpack resolver looks for custom webpack config in packages with jsnext:main #666

tf opened this issue Nov 14, 2016 · 20 comments · May be fixed by #994

Comments

@tf
Copy link

tf commented Nov 14, 2016

With a .eslintrc like

"settings": {
  "import/resolver": {
    "webpack": {
      "config": "webpack.karma.config.js"
    }
  }
},

when I import

import {takeEvery} from 'redux-saga';

I get the error

 Cannot find module '/path/to/project/node_modules/redux-saga/webpack.karma.config.js

When I remove the jsnext:main entry from the package.json inside node_modules/redux-saga, the error goes away. So I guess the plugin is somehow trying to find a Webpack config inside redux-saga to load resolve configuration for ES6 files? This might be fine, looking for my custom Webpack config does not make sense, though.

@k15a
Copy link
Contributor

k15a commented Nov 14, 2016

Did you tried ./webpack.karma.config.js?

@tf
Copy link
Author

tf commented Nov 14, 2016

@k15a Prefixing the path with ./ does not change anything.

@Voxis
Copy link

Voxis commented Jan 31, 2017

I'm using semantic-ui-react and encountered the exact same issue. Semantic-Org/Semantic-UI-React@1911865

@SpainTrain
Copy link

Can corroborate. Our workaround was to create a symlink with the default name that targets the desired custom config, e.g.,

webpack.config.js -> karma.webpack.config.js

@gwleclerc
Copy link

I found a workaround for this problem by converting my ".eslintrc" into a ".eslintrc.js" and use path.resolve as follow:

var path = require('path');

module.exports = {
  settings: {
    'import/resolver': {
      webpack: {
        config: path.resolve("./webpack.config.dev.js")
      }
    }
  },

@philostler
Copy link

@gwleclerc Tried your suggestion and didn't work for me

@arabold
Copy link

arabold commented Jul 22, 2017

Same issue here when using react-phone-number-input.

@wmertens
Copy link
Contributor

Perhaps this can be solved by mutating the configuration when the plugin first gets it, converting relative paths to absolute paths.

Ideally, it would do the conversion using the absolute path of the configuration file.

@wmertens
Copy link
Contributor

I tried putting the following line at https://github.com/benmosher/eslint-plugin-import/blob/master/resolvers/webpack/index.js#L65:

      if (configPath !== settings.config) settings.config = configPath

however, while that fixes CLI usage, it doesn't fix it in the editor. The problem is that there are many invocations, each having to do fix-up on first invacation, and somehow in my editor the node_module is the first invocation.

Billy- added a commit to Billy-/eslint-plugin-import that referenced this issue Jan 5, 2018
when a relative `configPath` is passed, it should be resolved relative to the source file, not the imported file.
@Billy-
Copy link

Billy- commented Jan 5, 2018

I may have jumped the gun a bit with the PR above (variable names are confusing!)...

So my next question becomes, why is eslint trying to resolve those files anyway when (by default) it should ignore any files in node_modules?

By logging out the source and file passed into the resolver, I can see that it is attempting to resolve imports from within files in node_modules:

// Just showing the last two logs before the error
{ file: 'my-project/src/store/modules/index.js',
  source: 'react-router-redux' }
{ file: '/my-project/node_modules/react-router-redux/es/index.js',
  source: './reducer' }

@ljharb
Copy link
Member

ljharb commented Jan 5, 2018

eslint-plugin-import most definitely does not ignore files in node_modules, because it's trying to verify the exports that you're importing from there. Separately, webpack may alias some of those out, so the resolver needs to run on them.

@Billy-
Copy link

Billy- commented Jan 5, 2018

Ah, I see. So would a valid fix for this issue be to simply check if we're in a node_module, and if so, ignore the configPath setting?

I think it's fair to say that when setting a configPath, you only want it to apply to your code, not any dependencies, right?

@Billy-
Copy link

Billy- commented Jan 5, 2018

Although I guess that would lead to inconsistent behaviour between absolute paths and relative, where absolute paths would always resolve a webpack config, relative ones would not in this case...

If you agree with my previous comment, would you also agree that it should apply to all cases (that is, if we're in a dependency; ignore configPath)?

@Billy- Billy- linked a pull request Jan 5, 2018 that will close this issue
@cjpete
Copy link

cjpete commented Jan 5, 2018

If it helps I'm also seeing this issue - there's a simple repository here showing the error:

https://github.com/cjpete/redux-saga-import-issue

@Andarist
Copy link

Andarist commented Jan 5, 2018

@Billy-
wouldnt this proposed solution work in clean manner?

Perhaps this can be solved by mutating the configuration when the plugin first gets it, converting relative paths to absolute paths.

@Andarist
Copy link

Andarist commented Jan 6, 2018

I've successfully narrowed down the problem (at least for redux-saga) to the reexports being used:

export { foo } from './internal';

It seems that this plugin follows them, tries to resolve ./internal, but to resolve it it tries to apply a webpack config rules. It tries to resolve a webpack config, but to find it it uses a relative path and since it's looking at a package in node_modules we have a problem.

On the other hand 'manual' reexports are treated as safe:

import { foo } from './internal';
export { foo }

and it doesn't try to dive into ./internal in such case.

I'd still advocate for the simplest solution - just resolve config path once to an absolute path and be done with it, WDYT @ljharb ?

@ljharb
Copy link
Member

ljharb commented Jan 6, 2018

I’m not sure i know enough about it to decide - i prefer to avoid webpack aliasing, personally.

It’s certainly worth a PR if tests can be made to pass.

@Billy-
Copy link

Billy- commented Jan 6, 2018

@Andarist I suppose yes it works, although as mentioned there is a caveat in that if the first file to be resolved is a node_module it doesn't work. I also think that mutating in this way is a bit of an antipattern..

wouldnt this proposed solution work in clean manner?

@benmosher
Copy link
Member

per discussion in #995: need to enhance resolver spec to include the linting file in order to properly load webpack config relative to the linting file and not deep imports

@aibolik
Copy link

aibolik commented Apr 9, 2019

@gwleclerc your workaround works for me :) Had a yaml before, changed to .eslintrc.js with usage of path module and it worked.

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