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

Strict mode - Detect duplicate UglifyPlugin #71

Open
bebraw opened this issue May 2, 2016 · 10 comments
Open

Strict mode - Detect duplicate UglifyPlugin #71

bebraw opened this issue May 2, 2016 · 10 comments

Comments

@bebraw
Copy link
Contributor

bebraw commented May 2, 2016

Just saw a case where a user was using -p and UglifyPlugin separately. Latter was used to set compress['drop_console']. The problem is that this will lead to double minification and it won't work as you might expect.

It would be good to be able to detect this case. Perhaps the same applies to some other plugins as well, but this would be a nice starting point.

@kentcdodds
Copy link
Collaborator

Good idea. I think it'd be great if there were a way for Webpack to prevent this from happening at all. Double minification is probably never desirable...

@bebraw
Copy link
Contributor Author

bebraw commented May 2, 2016

Speaking of minification, I have seen cases where people apply minification on already minified files. Usually that ends up in weirdness (not terminating builds!). That's one thing, but probably hard to detect.

@scotttball
Copy link

scotttball commented May 10, 2016

I actually spoke with @kentcdodds about this. I ran into this issue while analyzing the build time of a few applications that I was working with. I found out that -p was being used in the package.json and UglifyJsPlugin was also being specified in the webpack config. This caused webpack to hang on building ~9 min. When I removed -p from the package.json script, it reduced the time to just ~1:30 min.

Note: I had to also put OccurrenceOrderPlugin in the webpack config since -p included that as well. (not sure how important)

I think the problem is that the validator can't detect the whether the user is using -p or not (whether it be in the package.json or in the cli).

Adding a note to not use -p if the validator detects UglifyJsPlugin might be the best way to handle this.

@bebraw
Copy link
Contributor Author

bebraw commented May 10, 2016

Good point about -p. It would be a little difficult to get into that. It's possible, though, especially if you use the cli. #87 goes to this direction. We could detect -p etc. through that. The problem is that we would have to code a rule for each case. That's something I would rather avoid if possible.

Ideally we should be able to use the rules of webpack itself. We could perhaps do that by using bin/convert-argv.js (webpack 1) and pushing argv through that to see what kind of configuration it generates and go from there. Just a quick idea.

@kentcdodds
Copy link
Collaborator

Shouldn't we be able to get the args from process.argv?

@bebraw
Copy link
Contributor Author

bebraw commented May 10, 2016

Shouldn't we be able to get the args from process.argv?

If you invoke the validator inside configuration, that would work.

If we go through the CLI (#87), it will need a little different kind of handling as then we are looking at it from the outside (package.json scripts).

@kentcdodds
Copy link
Collaborator

Good point. Something's better than nothing though. But if we can handle it for both cases that's be great.

Honestly, this seems like something Webpack should warn about by itself...

@bebraw
Copy link
Contributor Author

bebraw commented May 10, 2016

Honestly, this seems like something Webpack should warn about by itself...

Absolutely. We are just putting some bandaid on it. ;)

The implementation doesn't look too bad. Both cases are fairly straight-forward to handle because we can piggyback on webpack's own logic here.

@jonathanglasmeyer
Copy link
Collaborator

Yep, sounds reasonable. More brittle script parsing, but I guess PRs to webpack itself aren't very likely to get through, or am I wrong?

@bebraw
Copy link
Contributor Author

bebraw commented May 10, 2016

Yep, sounds reasonable. More brittle script parsing, but I guess PRs to webpack itself aren't very likely to get through, or am I wrong?

I'm not so sure. We can definitely build an awesome prototype here and then negotiate. These kind of things are fairly simple in user space.

My thinking is that webpack core should drop all the flag stuff (less to maintain) but that's another discussion. 👍

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

No branches or pull requests

4 participants