refactor(build): Remove closure-make-deps and closure-calculate-chunks#7473
Conversation
Rewrite the getChunkOptions function to not use
closure-calculate-chunks, but instead just chunk the input files
(more or less) by subdirectory: first chunk is core/, second is
blocks/, etc.
This does make a material change to blockly_compressed.js,
because we end up feeding several empty modules that contain
only typescript interface declarations and which tsc
compiles to "export {};" in the input to Closure Compiler
(closure-calculate-chunks is smart enough to notice that
no other module depends on these), which results in ~1.7KiB of
superflous
var module$build$src$core$interfaces$i_ast_node_location_svg={};
declarations. This can be avoided by filtering such empty modules
out but that has been left for a future commit.
This adds the glob NPM package as a dev dependency, but gulp
and several other existing dev dependencies already depend on
it.
Build time is sped up by about a factor of 3x, due to removal
of the buildDeps step that was really slow:
$ time npm run build
before:
real 0m24.410s
user 0m16.010s
sys 0m1.140s
after:
real 0m8.397s
user 0m11.976s
sys 0m0.694s
Closure Compiler renames module globals so that they do not clash when multiple modules are bundled together. It does so by adding a "$$module$build$src$path$to$module" suffix (with the module object istelf being named simply "$module$build$src$path$to$module"). By changing the gulp.src base option to be build/src/ instead of ./ (referring to the repostiory root), Closure Compiler obligingly shortens all of these munged named by removing the "$build$src" infix, reducing the size of the compressed chunks by about 10%; blockly_compressed.js goes from 900595 to 816667 bytes.
- Add modulePath to compute the munged name of the entrypoint module from the entrypoint module filename, and use this instead of hard-coded chunk.exports paths. - Be more careful about when we are using poxix vs. OS-specific paths, especially TSC_OUTPUT_PATH which is regularly passed to gulp.src, which is documented as taking only posix paths. - Rename one existing variable modulePath -> entryPath to try to avoid confusion with new modulePath function.
|
I'm excited about this one! In the past we've had a few developers who exclusively use Windows report issues with our build system. Please tag in one or two of them to check that this still works on Windows. |
rachel-fenichel
left a comment
rachel-fenichel
left a comment
There was a problem hiding this comment.
I love this PR.
In addition to finding someone to test on Windows, it's worth packaging up a beta and testing it with examples and plugins to make sure nothing was magically depending on a detail of the build system.
Thanks!
I attempted to test this myself on Windows, but I don't know a lot about how Windows devs typically set up their machines. |
|
@yamadayutaka: You have been our main Windows developer when it comes build system testing. Can you pull this change (now merged to Also, I now have access to a Windows machine but am not sure how to set it up in a way that best represents how developers like you work. An explanation of how you have things set up (what did you install, and from where? Are you using WSL + bash or just Command Prompt? etc. etc.) would be very useful. |
|
Hi @cpcallen, I tried running The If you change the function renamings() {
- return runTestCommand('renamings', 'tests/migration/validate-renamings.mjs');
+ return runTestCommand('renamings', 'node tests/migration/validate-renamings.mjs');
}
I think it would be better to be able to build with PowerShell Propmpt instead of using WSL. |
3 participants
The basics
The details
Resolves
Fixes #7469
Proposed Changes
Rewrite the
getChunkOptionsfunction to not useclosure-calculate-chunks, but instead just chunk the input files (more or less) by subdirectory: first chunk iscore/, second isblocks/, etc.This does make a material change to
blockly_compressed.js, because we end up feeding several empty modules that contain only typescript interface declarations and whichtsccompiles toexport {};in the input to Closure Compiler(
closure-calculate-chunksis smart enough to notice that no other module depends on these), which results in ~1.7KiB ofsuperflous
var module$build$src$core$interfaces$i_ast_node_location_svg={};declarations. This can be avoided by filtering such empty modules out, but that has been left for a future commit.This adds the
globNPM package as a dev dependency, butgulpand several other existing dev dependencies already depend on it.Build time is sped up by about a factor of 3x, due to being able to skip the
buildDepsstep that was really slow:Remove the
buildDepstask.Remove the
$build$srcinfix from munged paths.Closure Compiler renames module globals so that they do not clash when multiple modules are bundled together. It does so by adding a
$$module$build$src$path$to$modulesuffix (with the module object itself being named simply$module$build$src$path$to$module).By changing the
gulp.srcbaseoption to bebuild/src/instead of./(referring to the repository root), Closure Compiler obligingly shortens all of these munged names by removing the$build$srcinfix, reducing the size of the compressed chunks by about 10%;blockly_compressed.jsgoes from 900595 to 816667 bytes.Compute module object munged name from entrypoint:
modulePathto compute the munged name of the entrypoint module from the entrypoint module filename, and use this instead of hard-codedchunk.exportspaths.TSC_OUTPUT_PATHwhich is regularly passed togulp.src, which is documented as taking only posix paths.modulePath->entryPathto try to avoid confusion with the newmodulePathfunction.Reason for Changes
Fewer dependencies, faster builds, simpler build config!
Test Coverage
npm testpasses; playground loads in compressed and uncompressed mode.