feat(binding): add active-related APIs to ModuleGraphConnection#13548
feat(binding): add active-related APIs to ModuleGraphConnection#13548
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5fb1f1212
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds active-state inspection APIs for ModuleGraphConnection (core + binding) so JS plugins can query whether a connection is active/conditional, plus a config test validating the behavior.
Changes:
- Expose
getActiveState(runtime)(binding) and surface non-boolean states as Symbols in@rspack/core. - Export
ConnectionState+ related Symbol constants from@rspack/core. - Add a new config test case
module-graph/get-incoming-connections-activeto validate active incoming connections.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rspack-test/configCases/module-graph/get-incoming-connections-active/used.js | Adds a used module for module-graph connection assertions |
| tests/rspack-test/configCases/module-graph/get-incoming-connections-active/unused.js | Adds an imported-but-unused module fixture |
| tests/rspack-test/configCases/module-graph/get-incoming-connections-active/index.js | Entry that imports used/unused modules |
| tests/rspack-test/configCases/module-graph/get-incoming-connections-active/rspack.config.js | Plugin-based test asserting getActiveState() results on connections |
| packages/rspack/src/exports.ts | Re-exports ConnectionState and Symbol constants |
| packages/rspack/src/ModuleGraphConnection.ts | Monkey-patches binding getActiveState() to map string states to Symbols |
| packages/rspack/src/ModuleGraph.ts | Ensures the ModuleGraphConnection patch is applied via side-effect import |
| crates/rspack_binding_api/src/module_graph_connection.rs | Adds get_active_state(runtime) N-API method and computes active-state in Rust |
| crates/node_binding/napi-binding.d.ts | Updates TS declarations for ModuleGraphConnection.getActiveState() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will degrade performance by 1.32%🎉 Hooray!
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | rust@persistent_cache_restore@basic-react-development |
26 ms | 26.3 ms | -1.32% |
| ⚡ | Simulation | bundle@basic-react-development |
348.1 ms | 344.6 ms | +1.02% |
Comparing fy/stupefied-noyce (c280360) with main (c88a8d5)
Footnotes
-
19 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Rsdoctor Bundle Diff AnalysisFound 6 projects in monorepo, 6 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 popular-libsPath:
📁 react-10kPath:
📁 react-1kPath:
📁 react-5kPath:
📁 romePath:
📁 ui-componentsPath:
Generated by Rsdoctor GitHub Action |
📦 Binary Size-limit
🎉 Size decreased by 4.72KB from 49.26MB to 49.26MB (⬇️0.01%) |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aa432bb1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Expose connection active state in the binding layer so JS plugins can inspect whether a module graph connection is active, conditional, or evaluate its full active state with a given runtime. - Add `active` getter (evaluates `is_active` with compilation context) - Add `conditional` getter - Add `getActiveState(runtime)` method returning state as string - Add `is_conditional()` public accessor on core `ModuleGraphConnection` - Add test case for active incoming connections
…'transitive-only' | 'circular' Remove `active` and `conditional` getters, keep only `getActiveState(runtime)` which returns `boolean | string` matching webpack's ConnectionState semantics.
- Rust binding returns `boolean | string` from `getActiveState`
- JS layer patches prototype to convert strings to Symbols:
- `Symbol("transitive only")` for TRANSITIVE_ONLY
- `Symbol("circular connection")` for CIRCULAR_CONNECTION
- Export TRANSITIVE_ONLY, CIRCULAR_CONNECTION, and ConnectionState type
from @rspack/core
- Remove getActiveState from napi ModuleGraphConnection class - Add getActiveState(connection, runtime) to JsModuleGraph napi class - Add ModuleGraphConnectionRef for passing connection as method argument - ModuleGraph.ts wraps binding method, converting strings to Symbols - ModuleGraphConnection.ts exports TRANSITIVE_ONLY, CIRCULAR_CONNECTION symbols and ConnectionState type
…ctly Use define_symbols\! macro to create CIRCULAR_CONNECTION and TRANSITIVE_ONLY symbols, and return them directly from Rust via custom ToNapiValue impl on JsConnectionState. No JS prototype patching needed — the napi method returns boolean | symbol natively.
After rebase, define_symbols! uses stringify!($cell) as export name, so CIRCULAR_CONNECTION_SYMBOL and TRANSITIVE_ONLY_SYMBOL are the actual binding export names. Update napi-binding.d.ts and re-exports.
df6c54e to
b9ea6bf
Compare
- Add CIRCULAR_CONNECTION_SYMBOL and TRANSITIVE_ONLY_SYMBOL to banner.d.ts so they persist across napi codegen - Add ConnectionState type alias in banner.d.ts - Fix prettier formatting in test rspack.config.js
exports_info_artifact and module_graph_cache_artifact are StealCell and may be stolen during certain compilation phases. Use try_read() with Default fallback instead of direct deref to prevent panic.
When exports_info_artifact is not available (stolen during finishModules hook), conditional connections cannot be properly evaluated. Return Active(true) as fallback since optimization has not run yet and all connections should be considered active.
Add CSS file and CssExtractRspackPlugin to test case. Verify that CssDependency outgoing connections return TRANSITIVE_ONLY symbol in processAssets phase where exports info artifact is available.
e4fa4d5 to
c280360
Compare
Summary
activegetter,conditionalgetter, andgetActiveState(runtime)method toModuleGraphConnectionbinding, allowing JS plugins to inspect connection active stateis_conditional()public accessor on coreModuleGraphConnectionget-incoming-connections-active) verifying active incoming connection propertiesThis is useful to debug side effects bailouts. For example we can visit all active incoming connections to know what module is included only by side effect connections.