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

Support extra plugins in config (such as UglifyJS) #4

Merged
merged 2 commits into from
Jul 4, 2017

Conversation

drewhamlett
Copy link
Contributor

No description provided.

@joshwiens
Copy link
Contributor

Why try to optimize a dev only DLL?

@drewhamlett
Copy link
Contributor Author

drewhamlett commented Jul 2, 2017

The way we are currently using DLL is speed up dev times as well as compile times on platforms such as Heroku. We update our production vendor as well as dev vendor locally. We went from 2 minute builds to 20 seconds, something that Common Chunks and Happy Pack couldn't come close too.

If you look at something like Stripe.com they are using DLL's in production as well.

@joshwiens
Copy link
Contributor

Interesting, I can't say I would have thought of that from a devops perspective :)

@cpsubrian
Copy link

Hate to just +1 something, but I have a similar use case. I'd like to commit my production DLL to reduce the processing time of Netlify's continuous deployment pipeline. We trigger redeploys from CMS webhooks so they happen a lot, and the faster the better.

@dlong500
Copy link

dlong500 commented Jul 2, 2017

@d3viant0ne I don't see any reason why the DllPlugin can't be used in production. Can anyone point out downsides? Especially in the context of this autodll project it would be great to get a simplified way to split code for both dev and production scenarios.

In fact, I could see this plugin as a starting point for wrapping other more complex scenarios covering CommonsChunkPlugin and dynamic imports as well.

@asfktz
Copy link
Owner

asfktz commented Jul 2, 2017

Thanks for the PR, @drewhamlett!
This is definitely a feature worth adding.

It looks good! I would like to double check it before I merge it.

Interesting, I always thought about DLLs as a development tool, but it does make sense.

whether or not it's a good practice, adding an option to pass plugins to the DLL Compiler config is a good idea.

Especially in the context of this autodll project it would be great to get a simplified way to split code for both dev and production scenarios.

I agree, @dlong500, the plugin should not be too opinionated. It's only meant to simplify the setup.

@cpsubrian
Copy link

cpsubrian commented Jul 2, 2017

Another option, rather than accepting plugins specifically, would be to allow passing a config object and just merge that in using webpack-merge. Then users could pass plugins (or anything else) (or even override default config).

@joshwiens
Copy link
Contributor

@dlong500 - I never said or suggested you couldn't or shouldn't. I had just personally never thought to do it.

All of my applications are containerized, the use case @drewhamlett outlined above isn't something I have ever run into which is why I asked about it.

@asfktz asfktz merged commit f6632ab into asfktz:master Jul 4, 2017
@sudo-suhas
Copy link
Collaborator

Any idea what would be the implications of trying to use it for both dev & prod?

@dlong500
Copy link

dlong500 commented Jul 5, 2017

The only issue I can see with production usage is if you were also using stuff like dynamic imports. I don't believe the DllPlugin architecture handles dynamic imports like CommonsChunkPlugin does. Ultimately I'd like to see a plugin that could start with what CommonsChunkPlugin can do now (including handling dynamic imports) and add all the DllPlugin features as well. Combine that with an easy configuration along with cache handling like this project does and I think that would be great for both dev and production.

@sudo-suhas
Copy link
Collaborator

sudo-suhas commented Jul 5, 2017

The bundle generated for dev and prod would be different right? If there is already cached bundle for dev, when I run prod build, will it overwrite? Should the user perhaps tweak his config so the bundles don't conflict?

@dlong500
Copy link

dlong500 commented Jul 5, 2017

You may be right. I'm realizing the plugin may not invalidate the cache if the only thing that changed was NODE_ENV. It might make sense to have a separate cache per NODE_ENV value.

As far as the bundles conflicting, that would be up to the developer to set proper actions based on NODE_ENV, but I think the bigger issue is making sure it doesn't use cached data generated under a different NODE_ENV.

@sudo-suhas
Copy link
Collaborator

Some notes could be added to readme addressing this. Needs some investigation to understand exact behavior too.

@viankakrisna
Copy link
Contributor

viankakrisna commented Jul 5, 2017

the cache invalidation is too aggressive right now, and not taking account of things outside of settings variables. It deletes all files in cacheDir if settings have been changed. I've opened #13 for the first pass of using hash and additional files to watch for validating cache. Can do more in getHash,storeHash and cleanup

@asfktz asfktz added this to the 0.0.7 milestone Jul 9, 2017
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