Skip to content

removed DedupePlugin#3266

Merged
sokra merged 1 commit intomasterfrom
feature/no-dedupe
Nov 15, 2016
Merged

removed DedupePlugin#3266
sokra merged 1 commit intomasterfrom
feature/no-dedupe

Conversation

@sokra
Copy link
Copy Markdown
Member

@sokra sokra commented Nov 10, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Feature

What is the current behavior? (You can also link to an open issue here)
DedupePlugin is available but often doesn't work correctly.
It has problems with harmony modules.

What is the new behavior?
DedupePlugin is removed

Does this PR introduce a breaking change?

  • Yes

If this PR contains a breaking change, please describe the following...

  • Impact:
  • Migration path for existing applications:
    Remove DedupePlugin when referenced directly

Copy link
Copy Markdown
Member

@TheLarkInn TheLarkInn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably include a deprecation message. I didn't see any in the diff. @bebraw what do you think?

@bebraw
Copy link
Copy Markdown
Contributor

bebraw commented Nov 10, 2016

@TheLarkInn 👍 for a deprecation message. That way people know to get rid of it. Always go with warnings in deprecations and then drop those in the next major version.

@Jessidhia
Copy link
Copy Markdown
Member

But does 2 count as the next major version? Should it get a deprecation message in webpack 1?

@bebraw
Copy link
Copy Markdown
Contributor

bebraw commented Nov 10, 2016

@Kovensky I guess the expectation is that people will try to run webpack 2 with the plugin enabled when they upgrade. So I would push the messages there and then drop in webpack 3.

@Jessidhia
Copy link
Copy Markdown
Member

@bebraw since it outright conflicts with harmony modules, I guess it would be better to turn it into a noop that just prints the deprecation warning (also warning that it does nothing anymore)...

@sokra
Copy link
Copy Markdown
Member Author

sokra commented Nov 10, 2016

@bebraw since it outright conflicts with harmony modules, I guess it would be better to turn it into a noop that just prints the deprecation warning (also warning that it does nothing anymore)...

I can do that...

it's no longer needed for npm3
and causes many issues
@sokra sokra force-pushed the feature/no-dedupe branch from 196024a to 91cbb4c Compare November 15, 2016 08:26
@sokra sokra merged commit 5b2afd0 into master Nov 15, 2016
@sokra sokra deleted the feature/no-dedupe branch November 15, 2016 10:26
@niieani
Copy link
Copy Markdown
Contributor

niieani commented Nov 15, 2016

As a kind of a replacement of the DedupePlugin, you can now use RootMostResolvePlugin from 'webpack-dependency-suite/plugins/root-most-resolve-plugin'. It's a module.resolve plugin that ensures Webpack uses dependencies from the root-most node_modules as long as they satisfy SemVer version dependency of the requesting module.

See more info.

@sokra
Copy link
Copy Markdown
Member Author

sokra commented Nov 15, 2016

or just npm dedupe

@niieani
Copy link
Copy Markdown
Contributor

niieani commented Nov 15, 2016

@sokra npm dedupe won't work with linked modules.

@jquense
Copy link
Copy Markdown
Contributor

jquense commented Nov 15, 2016

@sokra npm dedupe won't work with linked modules.

we are also in this boat. our app is part of a lerna repo, so npm dedupe doesn't work well there.

@tleunen
Copy link
Copy Markdown

tleunen commented Nov 24, 2016

After some tests, I'm seeing multiple version of a few modules/files. Like with lodash. In a failry big app with a lot of dependencies, I can see lodash/isFunction.js multiple times for example. One in the root, and a couple of times under some dependencies.

What's the strategy now to dedupe these dependencies? I've tried npm dedupe and it didn't work. I still have all of them in the bundle.

@niieani
Copy link
Copy Markdown
Contributor

niieani commented Nov 24, 2016

@tleunen use root-most-resolve-plugin.

@tleunen
Copy link
Copy Markdown

tleunen commented Nov 24, 2016

I don't see any changes with this plugin. I still see multiple lodash/isFunction.js

@niieani
Copy link
Copy Markdown
Contributor

niieani commented Nov 24, 2016

@tleunen Then it's quite possible that your package's dependencies have version-incompatible ranges with regards to lodash they want to use. I can add an option to root-most-resolve-plugin to skip the SemVer check when matched by some sort of an ignore-list.

But doing that can cause unexpected problems in case package A depends on lodash 2.x and package B on lodash 4.x, the method signatures might be incompatible.

Also, did you add the plugin to the correct place in the config file (to resolve.plugins , not plugins)?

@tleunen
Copy link
Copy Markdown

tleunen commented Nov 24, 2016

I did.
screen shot 2016-11-24 at 11 42 22 am
So looks like it's normal I have the version for react-smooth, because they are using the tilde instead of the caret. But I also have the one for recharts.
So instead of having 3 copies, I should have 2 copies with your plugin, right?

@niieani
Copy link
Copy Markdown
Contributor

niieani commented Nov 24, 2016

@tleunen yeah, you should have two. So you still have 3? It would be best if I could implement an "error margin" function, so you could say, convert all ~ to ^ to make the matching more loose.

stefanwille pushed a commit to stefanwille/react-boilerplate that referenced this pull request Dec 7, 2016
DedupePlugin doesn't work correctly. It can cause this exception:
"Uncaught TypeError: Cannot read property 'call' of undefined" -
webpack/webpack#959

In a recent version of webpack, the plugin was changed to do nothing at all: webpack/webpack#3266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants