neutrinojs / neutrino

Create and build modern JavaScript projects with zero initial configuration.
https://neutrinojs.org
Mozilla Public License 2.0
3.94k stars 213 forks source link

Scale back default minification in core presets #713

Closed timkelty closed 6 years ago

timkelty commented 6 years ago

There have been a number of issues since we began including @neutrinojs/minify (minimizing babel, styles, images) in core presets: #678, #673, #700

My preference would be to scale back what the core presets do by default, and instead point users to additional middleware they may want to use (image-minify, style-minify).

That said - we probably also need to make sure the minify middleware include an enabled option (defaulting to NODE_ENV === 'production'), so that users could just include them without having to configure them.

Especially if we go this route, I'm not convinced theres much value in keeping @neutrinojs/minify around. Seems more explicit to have @neutrinojs/web include @neutrinojs/babel-minify directly, and direct users to include @neutrinojs/image-minify and @neutrinojs/style-minify if they desire.

I can work on a PR for v9, but curious to hear what others think first.

/cc @edmorley @mozilla-neutrino/core-contributors

edmorley commented 6 years ago

Sorry for the delayed reply! Yeah I agree it would be good to scale back the default packages included with @neutrinojs/web, and let people include them as needed (I've been contemplating skipping web and using the component pieces individually recently for my downstream project, due to the number of things now included in it that we don't use).

timkelty commented 6 years ago

@edmorley anything besides what is mentioned here (image/style-minify) you might want considered for removal from web?

edmorley commented 6 years ago

It's tricky, because I'm sure some of the things mozilla/treeherder doesn't use, others do - such as:

(And then in things like @neutrinojs/karma we don't use any of the Chrome related dependencies, overriding them with Firefox instead; but that's a different story)

timkelty commented 6 years ago

To some degree it matters if you're talking about the requiring dependencies themselves, or whether anything is actually done with them.

For example, a config of {html: false, manifest: false} would disable those features, though those packages would still be deps.

edmorley commented 6 years ago

I guess a bit of both. (In part depending on how large the deps are, and whether they contain compiled code that takes longer to build plus triggers the frustrating yarnpkg/yarn#932)

timkelty commented 6 years ago

Yep, I think that's mostly a problem for things like imagemin which has lots of hefty binaries to compile

edmorley commented 6 years ago

neutrino-dev master is now open for v9-alpha breaking changes, should anyone get a chance to tackle this one... :-)

timkelty commented 6 years ago

@edmorley I'm hoping to in the next couple weeks!

edmorley commented 6 years ago

@timkelty, I was contemplating starting this today - just checking I wasn't duplicating anything you had locally?

timkelty commented 6 years ago

@edmorley I haven't yet, unfortunately. I'll be available to help this week!

edmorley commented 6 years ago

I've opened PR #815 to split up the minify preset, and in so doing remove the web preset's dependency on image-minify. The PR leaves style-minify enabled by default - I was thinking it would be best to evaluate that post webpack 4 PR merge - since we'd actually be comparing the real performance when combining with mini-css-extract-plugin vs the older extract-text-webpack-plugin.

edmorley commented 6 years ago

Image minification was moved out of the web preset in #815 and similarly for style minification in #934. As such, now just JS minification is enabled by default.