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

Missing edge for multiple targets #8854

Merged
merged 2 commits into from
Feb 27, 2023
Merged

Missing edge for multiple targets #8854

merged 2 commits into from
Feb 27, 2023

Conversation

AGawrys
Copy link
Contributor

@AGawrys AGawrys commented Feb 24, 2023

↪️ Pull Request

This PR fixes the "missing edge" error when using multiple targets. A change had to be made in mutablebundleGraph because the way the current default bundler handles multiple targets is by producing an intermediate graph for each target and then mutating the assetgraph x times.

As a result, we attempted to add the same bundleGroup twice, which has an edge removal. We need to allow that edge to potentially already not exist.

This also raises a follow up question, are we doing duplicate work in the bundler for multiple targets? If two targets shared a lot of the same assets, do we need to decorate those nodes more than once or not? (For now it seems okay but something to think about)

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@AGawrys AGawrys changed the title Agawrys/missing edge targets Missing edge for multiple targets Feb 24, 2023
@parcel-benchmark
Copy link

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.53s -6.00ms
Cached 340.00ms +11.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

React HackerNews ✅

Timings

Description Time Difference
Cold 10.48s -89.00ms
Cached 418.00ms +2.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description Time Difference
Cold 1.85m +289.00ms
Cached 2.00s +54.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Three.js ✅

Timings

Description Time Difference
Cold 8.64s +4.00ms
Cached 258.00ms -4.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@AGawrys AGawrys merged commit 0cf2aff into v2 Feb 27, 2023
@AGawrys AGawrys deleted the agawrys/missing-edge-targets branch February 27, 2023 18:00
marcins pushed a commit to marcins/parcel that referenced this pull request Jul 14, 2023
* upstream/v2:
  Missing edge for multiple targets (parcel-bundler#8854)
  Split large runtime manifest into separate bundles (parcel-bundler#8837)
  Improvements to new resolver (parcel-bundler#8844)
  Fix published files for resolver
  New resolver implementation in Rust (parcel-bundler#8807)
  Update yarn.lock (parcel-bundler#8843)
  Bump napi-rs to latest (parcel-bundler#8838)
lettertwo added a commit that referenced this pull request Nov 6, 2023
* upstream/v2: (128 commits)
  [webextension] Add support for `chrome_style` (#8867)
  Switch to SWC minifier by default (#8860)
  Use BitSet for bundler intersections (#8862)
  best key logic truncating package names (#8865)
  Add support for loadConfig to resolver plugins (#8847)
  Missing edge for multiple targets (#8854)
  Split large runtime manifest into separate bundles (#8837)
  Improvements to new resolver (#8844)
  Fix published files for resolver
  New resolver implementation in Rust (#8807)
  Update yarn.lock (#8843)
  Bump napi-rs to latest (#8838)
  Support .proxyrc.cjs  (#8833)
  Sort global deps before injecting imports (#8818)
  Sort CSS module exports (#8817)
  fix: add extra information to unique bundles (#8784)
  Don't blow up HMR when <link />s don't have hrefs (#8800)
  v2.8.3
  Changelog for v2.8.3
  Address bug by updating an asset reference and merge conditions (#8762)
  ...
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.

Edge from X to Y not found! when using inline Sass and multiple targets
3 participants