Skip to content

Conversation

@mzgoddard
Copy link
Contributor

@mzgoddard mzgoddard commented May 1, 2018

Proposed Changes

  • Use src/index.js as the entry by default in tools building scratch-vm into a larger package
  • List src dependencies in the package dependencies category so they are installed for users of this package (scratch-gui for example)

Reason for Changes

This saves file space by excluding extra webpack boilerplate, excluding extra copies of dependencies and may allow for deeper optimizations.

As is, scratch-* packages build in their dependencies. Three such packages build in text-encoding: scratch-vm, scratch-storage, and scratch-gui. These three copies are each ~500KB of source before minification and compression. Importantly they are also duplicate execution paths, that build up 3 copies of the same runtime memory structures and 3 copies of compiled functions. There are two copies of scratch-svg-renderer's 1.8MB source depended on by scratch-render and scratch-gui.

Removing these duplicate copies brings the scratch-gui lib.min.js file size from (webpack 4's) 9.2MB (3.6 gzipped) to 7.1MB (2.8 gzipped). (Additionally with #1105 this further drops to 5.2MB (1.4 gzipped). The music sample js with webpack-4 branch's arraybuffer-loader is 2.0MB (1.4 gzipped))

With the webpack-4 branch this reduces runtime memory usage on a Samsung Chromebook Plus from 28MB (develop) to 21MB (webpack-4) to 18MB.

caveats

The main caveat for this change is that some building details would need repeating in some of the packages depending on other scratch packages like scratch-vm and scratch-gui. Packages with their module rules configurations are not automatically included in builds consuming that package. scratch-render for instance uses ify-loader on its linebreak and grapheme-breaker dependencies. scratch-vm and scratch-gui need to support handling this.

There are some ways that this could be resolved in the dependencies. Parts that need this extra handling could be prebuilt. Instead of building the entire package for use in later browser builds, the parts with webpack configuration details could be prebuilt and depended on by the package's source.

Some babel details can be handled by compiling to another build folder with modules individually compiled using the babel cli tool.

Probably the easiest option would be to inline the loader configuration details. Many binary files in the scratch packages have their loader stated in the require request. This could be extended to other dependencies like linebreak in scratch-vm to resolve this.

merging

A second minor caveat is this PR should be merged with its sibling PRs in each package or from top down, starting at scratch-gui. Directly related to build configuration packages using other scratch packages needed to handle cases building where the consumed packages have build details that are not already handled.

Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mzgoddard, I think we should do this! The only concern is what you listed in caveats, but luckily I think most of our tests for scratch-render are going to end up being integration tests on the built bundle, so this shouldn't be an issue — we'll just make the ify loader inline on those two requirements.

Use src/index.js as the entry by default in tools building scratch-vm
into a larger package. This saves file space by excluding extra webpack
boilerplate and may allow for deeper optimizations.

Depend on script dependencies for downstream webpack. For another
package to build in scratch-vm's dependencies without them already
being built into a consumed webpack build they need to be listed as
dependencies. This can benefit large projects that reuse the same
dependencies multiple times. Node will still use the main entry point
and its webpack build leaves the dependencies as external references so
it may reuse common modules in Node as well as in a build a browser
environment.
@rschamp rschamp force-pushed the dependents-no-boiler branch from a978308 to 4ceeebb Compare May 31, 2018 20:32
@rschamp rschamp merged commit 44b03fe into scratchfoundation:develop May 31, 2018
rschamp pushed a commit to rschamp/scratch-vm that referenced this pull request May 31, 2018
This would have caught the issue in scratchfoundation#1106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants