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

WIP: Per-source minification (DO NOT MERGE) #115

Closed
wants to merge 3 commits into from
Closed

Conversation

guybedford
Copy link
Contributor

Here's a proof of concept towards caching for minification in #46.

Because minification is done per-source, the output currently looks like:

module.exports =
/******/ (function(modules) { // webpackBootstrap
/******/ 	// The module cache
/******/ 	var installedModules = {};
/******/ ETC
/************************************************************************/
/******/ ([
/* 0 */
/***/ (function(module, exports, __webpack_require__) {
// minified line:
var bind=__webpack_require__(4),isBuffer=__webpack_require__(5),toString=Object.prototype.toString;
/***/ }),
/* 2 */
/***/ (function(module, exports, __webpack_require__) {

// another minified line:
var utils=__webpack_require__(0),normalizeHeaderName=__webpack_require__(26),DEFAULT_CONTENT_TYPE={"Content-Type":"application/x-www-form-urlencoded"}
// ETC
/***/ })
/******/ ]);
//# sourceMappingURL=index.js.map

It could be possible to hack the various webpack outputs to reduce the gaps here, but it definitely would be quite a hack.

The benefit at the end of the day would be per-source minification caching which would significantly speed up builds with minification.

Let me know if this seems like a worthwhile direction to continue along.

@codecov-io
Copy link

codecov-io commented Dec 5, 2018

Codecov Report

Merging #115 into master will increase coverage by 0.34%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   77.29%   77.64%   +0.34%     
==========================================
  Files          11       12       +1     
  Lines         511      519       +8     
==========================================
+ Hits          395      403       +8     
  Misses        116      116
Impacted Files Coverage Δ
src/index.js 93.47% <ø> (ø) ⬆️
src/loaders/terser-loader.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c289b28...b0a6294. Read the comment docs.

@guybedford guybedford force-pushed the terser-loader branch 2 times, most recently from 29e3efb to 1bc6261 Compare December 6, 2018 12:27
@timneutkens
Copy link
Member

Yeah so hacking those outputs would be very breaking when trying to upgrade webpack. However it's not like webpack's current output is the biggest part of minification work, so it makes sense to not minify those parts.

@sokra
Copy link
Member

sokra commented Dec 12, 2018

This might be an interesting source if you looking for performance vs size tradeoffs too: https://github.com/terser-js/terser#terser-fast-minify-mode

@guybedford
Copy link
Contributor Author

@sokra good point, added in #161. Am I right in thinking Webpack already does the simple optimizations which would explain why we're seeing absolutely no difference?

@guybedford guybedford closed this Dec 17, 2018
@guybedford guybedford deleted the terser-loader branch December 17, 2018 22:24
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.

4 participants