-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Change edge types to numbers #6126
Conversation
|
Benchmark ResultsKitchen Sink ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. React HackerNews ✅
Timings
Cold Bundles
Cached BundlesNo bundle changes detected. AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundle changes detected. |
You may want to look at |
packages/core/core/src/Graph.js
Outdated
@@ -6,18 +6,19 @@ import type {TraversalActions, GraphVisitor} from '@parcel/types'; | |||
import assert from 'assert'; | |||
import nullthrows from 'nullthrows'; | |||
|
|||
export type GraphOpts<TNode, TEdgeType: string | null = null> = {| | |||
type NullEdgeType = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should not support the null edge type at all, and just have users/extenders always specify the types of edges the way they do nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…-edgetypes # Conflicts: # packages/core/core/src/BundleGraph.js # packages/core/core/src/dumpGraphToGraphViz.js # yarn.lock
Closing this for now since this will be involved in a different task for buffer-backed edges in the graph. |
Why did we close this? Couldn't this be landed separately from upcoming work? |
I think I closed this because we thought we were close to opening a PR for the main work 😞 which includes the numeric edge types, and landing this by itself didn't have any significant performance impact. |
It will make the work reviewable though, yeah? |
Yeah good point, I reopened it. |
a97bcc1
to
c0b7651
Compare
* v2: (42 commits) Remove dead link to docs in diagnostic (#6913) Fix engines.parcel in SVG packager (#6911) Use imported Readable flow type instead of global (#6910) Use non-deprecated SVGO options in HTMLNanoOptimizer (#6785) Use yarnpkg registry for all the things (#6908) Fix issue about loading configs from tsconfig.json (#6881) Add `@section` to Compressor type (#6885) Parcel API improvements (#6866) Use stream-browserify for polyfilling instead (#6863) Add support for compressor plugins (#6776) Support for inline style attributes and inline scripts in SVG (#6797) Bump less (#6852) Make sure (non-React) SVGs are in separate bundles (#6757) Add support for injecting manifest into service workers (#6798) Fix 'does not export default' error with scope hoisting and url/worklet pipeline (#6803) Remaining cargo clippy fixes (#6829) Lazily install sharp only when needed (#6816) More cargo clippy suggestions (#6811) Don't emit runtime manifest for inline child bundles (#6807) Apply suggestions from rust-clippy (#6256) ...
c0b7651
to
0b0a475
Compare
This should be able to land before #6922 I believe. Looks like there's a small lint issue. |
Should be fixed now 😄 |
* v2: Upgrade Flow to 0.160.1 (#6964) Only use error overlay if there's a document (#6960) Don't fail when HTML tags are implied (#6752) Reorder resolveOptions() env priority (#6904) Change edge types to numbers (#6126) Bump swc (#6848) Print diagnostics for scope hoisting bailouts at verbose log level (#6918)
↪️ Pull Request
Eventually we'll work on moving the edge list in the Graph to a buffer, but we need to change edge types to numbers first in order to do that.
🚨 Test instructions
✔️ PR Todo