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.

Sign up for GitHub

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

Jump to bottom

JS runtime improvements #6531

Merged
merged 8 commits into from Jul 7, 2021
Merged

JS runtime improvements #6531

merged 8 commits into from Jul 7, 2021

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jun 30, 2021 •
edited

Some improvements to the JS runtime to reduce bundle size and improve caching.

  • The ESM output format now uses the runtime manifest in the entry bundle just like the global output format. This should result fewer cascading cache invalidations.
  • We use new URL(relativePath, import.meta.url) in ESM output when targeting browsers supporting import.meta.url natively. This avoids needing to include the runtime that attempts to determine the script url by parsing an error stack.
  • The relative path utility function is removed. Rather than the manifest storing the full path from the target dist dir, it only stores the base name of the bundle since the part of the path that's most likely to change is just the hash. The relative path part (if any) is computed at build time and inlined into the bundle to avoid the need for this large-ish function.
  • Don't include worker url helper and bundle url helper in library builds. Fixes Workers don't work in library mode #6261.

To do:

  • tests
  • questions below

@height
Copy link

height bot commented Jun 30, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@parcel-benchmark
Copy link

parcel-benchmark commented Jun 30, 2021 •
edited

Benchmark Results

Kitchen Sink 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

React HackerNews ✅

Timings

Description Time Difference
Cold 8.36s -101.00ms
Cached 412.00ms -17.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 487.47kb -591.00b 🚀 4.80s -25.00ms
dist/logo.1e014c76.png 274.00b +0.00b 191.00ms -13.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 487.47kb -591.00b 🚀 4.77s -23.00ms
dist/logo.1e014c76.png 274.00b +0.00b 220.00ms +27.00ms ⚠️

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js ✅

Timings

Description Time Difference
Cold 6.10s +140.00ms
Cached 349.00ms -6.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

devongovett
devongovett commented Jul 4, 2021
View reviewed changes
packages/runtimes/js/src/JSRuntime.js
@@ -181,17 +181,9 @@ export default (new Runtime({
}),
);

if (bundle.env.outputFormat === 'commonjs' && mainBundle.type === 'js') {
Copy link
Member Author

Choose a reason for hiding this comment

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

This came from #4744 but I had to revert it because it breaks URL dependencies to a JS file in CJS output. I think the real solution to what the PR was trying to accomplish would be to make the dependency async.

@devongovett
Copy link
Member Author

For libraries, new Worker(new URL('worker.js', import.meta.url)) now becomes new Worker(new URL('HASH', import.meta.url)) during transformation, and the HASH is replaced with the final URL during packaging similar to URL dependencies in other languages like CSS. This way the output is statically analyzable, unlike our previous output of new Worker(require('some-worker-runtime')). That output is still used for non-libraries though, because we want to be able to use the bundle manifest.

@devongovett devongovett marked this pull request as ready for review July 5, 2021 04:14
@jessabyrne jessabyrne mentioned this pull request Jul 7, 2021
Closed
@devongovett devongovett merged commit ba548ef into v2 Jul 7, 2021
@devongovett devongovett deleted the js-runtime-improvements branch July 7, 2021 23:47
@devongovett devongovett mentioned this pull request Jul 10, 2021
Closed
lettertwo added a commit that referenced this pull request Jul 13, 2021
@lettertwo
Merge branch 'v2' into lettertwo/buffer-backed-graph
3a6eec7 
* v2: (34 commits)
  Wrap assets recursively when any incoming dependency is wrapped (#6572)
  Improvements for library targets (#6570)
  Diagnostic for undeclared external dependencies in library builds (#6564)
  More bugs (#6567)
  Don't require `url:` for image transformer (#6565)
  Remove 'Name already registered with serializer' error (#6566)
  Fix live bindings and `this` of external CommonJS modules (#6548)
  JS runtime improvements (#6531)
  Make sure the absolute path isn't contained in the cache (#5900)
  Adds '@parcel/diagnostic' to dependencies (#6563)
  Disable workers with string literals and improve diagnostics (#6536)
  Bug fixes (#6541)
  Don't attempt to resolve URLs starting with '#' (#6504)
  Correctly set worker's output format if not support by environment (#6534)
  Babel: Recognize peerDependencies in isJSX (#6497)
  fix setHeaders ordering on dev server (#6500)
  Graph: Remove Node interface (#6530)
  Fix TS build script for old Node versions (#6526)
  Improve library targets (#6517)
  Fix TypeScript and other sourcemaps by always creating an initial sourcemap (#6472)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wbinnssmith wbinnssmith wbinnssmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Workers don't work in library mode
3 participants
@devongovett @parcel-benchmark @wbinnssmith