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

Wrap assets recursively when any incoming dependency is wrapped #6572

Merged
merged 1 commit into from Jul 9, 2021

Conversation

devongovett
Copy link
Member

Fixes #6519.

This ensures that we wrap all assets recursively when any incoming dependency is wrapped. Previously, we'd only wrap the asset the dependency pointed to, not necessarily its dependencies as well. In the test case, a has a circular dependency on b -> a. c also depends on a, but forces a to be wrapped by requiring it inside a function. However, b was left unwrapped, resulting in a parcelRequire call to a that ran before a's other dependencies which are still unwrapped –💥. Basically, if an asset is wrapped, all of its children also have to be wrapped, but this wasn't true in some cases due to DFS stopping when it hits a node its seen already. Now we check all incoming dependencies for each asset rather than relying on the context when hitting dependency nodes.

@height
Copy link

height bot commented Jul 9, 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

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.30s -119.00ms
Cached 417.00ms +4.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/logo.1e014c76.png 274.00b +0.00b 219.00ms +29.00ms ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/logo.1e014c76.png 274.00b +0.00b 187.00ms -36.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 5.98s +3.00ms
Cached 367.00ms -4.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@devongovett devongovett merged commit e8c0c95 into v2 Jul 9, 2021
@devongovett devongovett deleted the fix-aws-sdk branch July 9, 2021 19:28
lettertwo added a commit that referenced this pull request Jul 13, 2021
* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws-sdk broken: Cannot read property 'prototype' of undefined
3 participants