Skip to content

Add support for Webpack 2 renamed modules key #319

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

Closed
wants to merge 1 commit into from
Closed

Add support for Webpack 2 renamed modules key #319

wants to merge 1 commit into from

Conversation

pascalduez
Copy link

@pascalduez pascalduez commented May 8, 2016

Hi,

just wanted to start the ball rolling, there most likely will be more work needed to fully support Webpack 2, but that very simple addition is already doing it for me on a project with custom import paths.

Rough changelog: https://gist.github.com/sokra/27b24881210b56bbaff7#resolving-options

If you feel that's okay for a start, I'll update the tests ?

Refs #265

@benmosher
Copy link
Member

I like the simplicity. I don't have any Webpack 2 projects to check against, so I'll take your word for it. 😎

New tests would be much appreciated. Probably a clone of the existing moduleDirectories tests would be a good start, assuming the semantics match.

@benmosher
Copy link
Member

Heads up: this will probably not be mergeable after I merge #345.

I think it will make more sense to make a webpack-2 resolver, see my notes on #345.

@michael-wolfenden
Copy link

@benmosher

In webpack2, resolve.root has been merged into the modules option, however the commit doesn't handle this.

webpack

resolve: {
    root: PATHS.appDir
}

webpack2

resolve: {
    modules: [
        'node_modules',
        PATHS.appDir,
    ],
},

I had to add the following code to make this work, the assumption being that root resolves are absolute paths

// Webpack >= 2
var rootPaths = get(webpackConfig, ['resolve', 'modules'], []).filter(path.isAbsolute);
paths = paths.concat(rootPaths)

@gausie
Copy link

gausie commented Jun 27, 2016

Now that #345 is merged, how are you feeling about this @benmosher?

@benmosher
Copy link
Member

benmosher commented Jun 27, 2016

I think I'd probably opt for a webpack2 resolver, mostly because there is a factory (in enhanced-resolve v2+) to build resolve from config that would be nice to leverage for ongoing correctness.

I can maintain it here (another PR, anyone?), or someone in the community can maintain it independently. Either way works for me.

I don't think I would keep this duck-typed version. I'm not sure how it would integrate with the current webpack resolver.

@pascalduez
Copy link
Author

Let's close this and do it the proper way :)

@pascalduez pascalduez closed this Jun 27, 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.

4 participants