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

Add plugins support for webpack resolver #320

Closed
wants to merge 5 commits into from
Closed

Add plugins support for webpack resolver #320

wants to merge 5 commits into from

Conversation

le0nik
Copy link
Contributor

@le0nik le0nik commented May 8, 2016

In my project, i use webpack plugin, that modifies path. I'm sure, that so do many other webpack users. For now I have to fork the webpack resolver and apply the plugins inside the resolve function.

This PR offers an extension point for users to apply plugins. I don't think it's possible at this point to apply plugins automatically, so user will have to do it manually for now. Here's how that would work:

in webpack.config.js

module.exports = {
  ...,
  plugins: [
    new NormalModuleReplacementPlugin(/\/app\//, '/newApp/')
  ]
};

in .eslintrc.js

module.exports = {
    settings: {

      'import/resolver': {

        webpack: {
          config: './webpack.config.js',
          plugins: [
             (source, { originalSource, rawSource }) => {
               if (/\/app\//.test(source)) {
                 source = source.replace(/\/app\//, '/newApp/');
               }
             }
          ]
        }

      }
    }
};

The user is expected to mutate source and/or path object.
Each plugin receives 2 arguments:

  • source - is the path to the imported file so far(after resolving alias and previous plugins in array)
  • info is an object with two properties:
    • originalSource - is the original path to file, as seen in the import/require statement itself
    • rawSource - is the same as originalSource, but with loader stripped out

There should be a way for the user to share the plugin options(like resourceRegExp and replacer function) between the eslint in webpack configs, but I haven't figured out the best way to do it yet.

Feedback is welcomed!

TODO:

  • Add Documentation

@le0nik le0nik changed the title Add plugins support for webpack resolver [Proof of concept] Add plugins support for webpack resolver May 8, 2016
@benmosher
Copy link
Member

I think plugins could be interesting, but I'd like to avoid having to duplicate config. If it's matching what is configured for Webpack already, I'm hesitant to do anything that doesn't use the configured plugins, without explicit (re-)reference in the ESLint config.

I'm not sure I understand exactly what you are proposing, though.

If the intent is to support Webpack resolver plugins, I would like someone to spend some time to evaluate using sokra's enhanced-resolve directly, first. I'm assuming this is doable, and that it would reduce the Webpack resolver to something much more like the Node resolver.

Would also be more future-proof, presumably; assuming the API doesn't change much, it could even declare enhanced-resolve as a peer dependency and use whatever version the installed Webpack comes with.

Does that make sense? Or have I misunderstood your intent?

@le0nik
Copy link
Contributor Author

le0nik commented May 8, 2016

@benmosher I saw your enhanced-resolver tracking issue and it's going to be awesome, when it lands!

I wanted to propose some temporary hacky solution, that would allow users to use plugins today(even though the duplication is a real issue here).

In my case, I had to rewrite the source path after the alias was resolved(NormalModuleReplacementPlugin only allows you to rewrite the original source path). So i wrote my own webpack plugin and it works, but now I have lots of errors from eslint, because plugins aren't yet supported and there's no point in the resolve function where I could do the replacement manually(which is the essence of this PR's proposal), so I'm left with forking eslint-import-resolver-webpack and maintaining the fork, where I replace the path in the resolve() function.

I thought it might be a relatively common case for other users, too.

Maybe I'm wrong about it. Anyway, this PR is just a proposal of a temporary solution for the problem. I'll totally understand, if you decide to reject it.

Although maybe there's something you or someone else can figure out about the duplication problem, before the support for enhanced-resolver lands.

@benmosher
Copy link
Member

Fair enough. Makes sense as a use case. I want to take another look at leveraging enhanced-resolve before merging this, though. If it's not a huge deal to get first-party support for the actual plugins, I want to start there.

Will try to take a look this week (or happy to hear from anyone else that is willing to take a stab at it).

@benmosher
Copy link
Member

Just pushed enhanced-resolve branch that uses actual Webpack enhanced-resolve. Would you be up for attempting to support plugins within this branch by apply-ing them in createResolver?

This is what I used as a guide: https://github.com/webpack/webpack/blob/v1.13.0/lib/WebpackOptionsApply.js#L321

It's not obvious how plugins get applied to resolvers, from this, but I suspect one of the compiler.applyPlugins .applys them to each of the resolvers (normal, etc.).

@benmosher
Copy link
Member

Closing for now. We can re-open or open a new one, as needed.

@benmosher benmosher closed this Jun 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants