feat(js-themes-toolkit): adds js minification step (Fixes #364) #365
Conversation
In production mode, `terser` will be invoked to minify `js` resources in a theme after every other processing has taken place. Terser options can be overriden by declaring a `terser` object in the `package.json` file of your theme. By default, it is invoked with defaults configuration. Sourcemaps are enabled by default and handled outside of terser if the minification takes place.
|
Oh, this fixes #364 |
|
@izaera, wouldn't mind you looking at this and merging/releasing if looks good. Otherwise, I'll get my hands dirty with the whole process 😉 |
| source-map "~0.7.2" | ||
| source-map-support "~0.5.19" | ||
|
|
||
| [email protected]: |
There was a problem hiding this comment.
This makes me super sad... the fact that gulp-terser requires [email protected] and yet we apparently use 4.8.0 befuddles me 😂
There was a problem hiding this comment.
html-webpack-plugin (last published 10 days ago 🎉) and more directly html-minifier-terser (last published 7 months ago 😢) are the main culprits:
yarn why terser 0.29s [wincent/fix-ci] ~/code/liferay-frontend-projects
yarn why v1.22.5
[1/4] 🤔 Why do we have the module "terser"...?
[2/4] 🚚 Initialising dependency graph...
warning Missing version in workspace at "/Users/greghurrell/code/liferay-frontend-projects/projects/js-themes-toolkit", ignoring.
[3/4] 🔍 Finding dependency...
[4/4] 🚡 Calculating file sizes...
=> Found "[email protected]"
info Has been hoisted to "terser"
info Reasons this module exists
- "workspace-aggregator-2b18f523-66b8-4ae1-9a79-f299cab4ade8" depends on it
- Hoisted from "_project_#@liferay#npm-scripts#terser"
- Hoisted from "_project_#terser-webpack-plugin#terser"
info Disk size without dependencies: "2.95MB"
info Disk size with unique dependencies: "3.94MB"
info Disk size with transitive dependencies: "3.95MB"
info Number of shared dependencies: 4
=> Found "gulp-terser#[email protected]"
info This module exists because "_project_#liferay-theme-tasks#gulp-terser" depends on it.
info Disk size without dependencies: "2.61MB"
info Disk size with unique dependencies: "3.6MB"
info Disk size with transitive dependencies: "3.61MB"
info Number of shared dependencies: 4
=> Found "html-minifier-terser#[email protected]"
info This module exists because "_project_#@liferay#npm-scripts#html-webpack-plugin#html-minifier-terser" depends on it.
info Disk size without dependencies: "1.86MB"
info Disk size with unique dependencies: "2.85MB"
info Disk size with transitive dependencies: "2.87MB"
info Number of shared dependencies: 4
✨ Done in 0.96s.
And people wonder why I am so grumpy about dependencies...
| 'build:fix-at-directives', | ||
| 'build:r2', | ||
| 'build:copy:fontAwesome', | ||
| 'build:minify:js', |
There was a problem hiding this comment.
Wait until the very end and then minify all the things. Keep in mind that we don't have a babel step ourselves, but people could configure it on their own, so chances are we're going to break things if that's the case (at least for sourcemaps).
bryceosterhaus
left a comment
bryceosterhaus
left a comment
There was a problem hiding this comment.
This seems fine to me, although this is also my first time ever looking at the js-themes-toolkit codebase... So don't put too much confidence in my thoughts here
jbalsas
changed the title
|
Thanks @bryceosterhaus! @izaera, this looks good to you? We're merging with merge commits here, aren't we? Not entirely sure about the process... it's been a while 😂 |
Happens to all of us 🙄 |
projects/js-themes-toolkit/packages/liferay-theme-tasks/lib/project/options.js
Show resolved
Hide resolved
|
@jbalsas This LGTM. Regarding merge, we use merge commit (not rebase), yes 👍 . |
|
I'll merge now and release in a while. |
Release notes for that:
https://github.com/liferay/liferay-frontend-projects/releases/tag/liferay-theme-tasks%2Fv10.2.0
Main point of interest is that it adds a JS minification step to the
theme build:
#365
When you do this, plz rewrite without gulp. kthxbye. |
4 participants
Hey @bryceosterhaus, @wincent, my gulp fu is pretty rusty (or my fu in general), but I think this should do it as far as minifying JS in themes goes.
Everytime I get back to this I get an intense urge of rewriting the whole thing, but instead, I end up adding yet another thing. One day, though...