Skip to content

Conversation

@ScriptedAlchemy
Copy link
Contributor

  • feat: support async startup promise across formats
  • fix(runtime): avoid duplicate export for mf async startup

Copilot AI review requested due to automatic review settings October 16, 2025 19:48
@netlify
Copy link

netlify bot commented Oct 16, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit ba6e99b
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/6906eeb5ab98a80008ee73c0

@github-actions github-actions bot added the release: feature release: feature related release(mr only) label Oct 16, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for async startup promises across different module formats and fixes duplicate exports in module federation async startup scenarios.

  • Adds async startup promise support for multiple module formats
  • Fixes duplicate export issues in module federation async startup runtime
  • Updates test snapshots to reflect webpack runtime changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025

📦 Binary Size-limit

Comparing ba6e99b to chore(deps): update dependency @biomejs/biome to ^2.3.2 (#12055) by renovate[bot]

❌ Size increased by 27.13KB from 47.85MB to 47.87MB (⬆️0.06%)

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 16, 2025

CodSpeed Performance Report

Merging #11899 will not alter performance

Comparing feature/async-startup-runtime-promise (ba6e99b) with main (1ad3537)

Summary

✅ 17 untouched

ScriptedAlchemy and others added 15 commits October 16, 2025 17:37
…t dependencies

- Add defensive safeguard to remove current chunk ID from chunks_ids
- Ensures direct execution is used when no chunk dependencies exist
- Fixes hundreds of snapshot test failures
- Aligns with enhanced implementation behavior

The issue was that chunks_ids could contain the current chunk ID after
get_all_chunks() traversal, preventing the condition 'chunks_ids.is_empty()'
from being true even for simple entries with no dependencies.

Test results:
- TreeShaking suite: All 82 tests pass
- Total passing: 5466 tests (up from ~5000)
- Remaining 17 failures are unrelated runtime issues with split chunks
- Fix path resolution in 1-container-full test to handle both CJS and ESM execution contexts
- Revert chunk-matcher to main version, removing experimental async startup changes
- Sync WASI binding exports (auto-generated)

These test fixes improve compatibility with dual-bundle execution contexts
and reduce test failures related to the async startup implementation.
Pass `self.async_chunk_loading || async_enabled` to runtime modules instead of
just `async_enabled`. This ensures projects using async chunk loaders (like
`chunkLoading: "async-node"`) continue to work correctly even when MF async
startup is disabled.

Previously, the code only passed `async_enabled` which would be false for
non-MF projects, causing them to get synchronous startup code that would
execute entries before their chunks finished loading.
Allow async startup when mfAsyncStartup is explicitly true, even for chunks with container entries. Only disable async startup for containers when it's implicitly enabled through chunk handlers.
Previously, enhanced mode would only set fallback error functions for
__webpack_require__.f.consumes, __webpack_require__.f.remotes, and
__webpack_require__.I instead of including the actual handler implementations.
This caused runtime errors when async startup was enabled.

Now enhanced mode includes the actual handler code:
- ConsumeSharedRuntimeModule includes consumesCommon.js, consumesInitial.js, and consumesLoading.js
- RemoteRuntimeModule includes remotesLoading.js
- ShareRuntimeModule includes initializeSharing.js

Also adds container entry detection to prevent async startup for container.js
files while allowing it for host files (main.js) that consume remotes.

Test results improved from 14 failures to 5 failures.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…pping

Refactored the runtime chunk mapping logic to handle multi-runtime scenarios
more robustly. The plugin now builds a comprehensive runtime-to-chunk map
that considers all chunks with runtime, not just entry chunks.

Key changes:
- Build runtime_chunk_map from all chunks with runtime, not just entries
- Improved module hoisting to handle complex runtime scenarios
- Added fallback to hoist to all runtime chunks if no specific runtime found
- Simplified chunk cleanup logic (removed chunk removal for now)

This addresses issues with module hoisting in Module Federation scenarios
where multiple runtime chunks need to share hoisted modules.
…lugin

The runtime chunk detection was broken because it used chunk.runtime()
which returns the runtime SPEC, not whether the chunk IS a runtime chunk.

Use get_chunk_graph_entries() which directly returns actual runtime chunks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ScriptedAlchemy
Copy link
Contributor Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

ScriptedAlchemy and others added 18 commits October 27, 2025 21:37
- Make container entry chunk skip conditional on mf_async_startup experiment
- Ensure container entries get proper runtime requirements when async startup is disabled
- Fix Promise.all wrapping for federation runtime initialization

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix container entry chunk handling for non-async startup mode
- Fix ESM library to include EsmExportImport dependencies
- Fix EmbedFederationRuntimeModule to wrap __webpack_require__.x for async startup
- Add federation initialization call to entry chunks in async mode
- Remove invalid __webpack_require__.X() call from delegating entry chunks

This resolves the following test failures:
- container-1-5/1-container-full
- All ESM output test failures (5 tests)
- Partially fixes container-1-5/2-async-startup-sync-imports (CJS working, ESM still failing)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ing, ESM needs fix)

- Implemented Promise-based startup wrapper for entry chunks with async federation
- CJS version works correctly with Promise.resolve().then() pattern
- ESM version has issue: import statements can't be wrapped in functions
- ESM needs different pattern: const __webpack_exports__Promise with async arrow functions

Current status:
- 1632/1633 tests passing
- 1 ESM test failing due to import statement placement

Next steps:
- Need to detect ESM vs CJS output format
- Use different wrapper pattern for ESM (async IIFE with top-level await)
- Or delegate to JavaScript plugin for proper ESM handling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…tests passing)

- Implemented Promise-based wrapper for CJS entry chunks with async federation
- Added ESM detection via compilation.options.output.module
- CJS version fully working with Promise.resolve().then() pattern
- ESM version partially working (needs import/execution separation)

Test Results:
- ✅ 1635/1636 tests passing (99.94%)
- ✅ All container and ESM output tests passing
- ❌ 1 ESM async startup test failing (import statement placement issue)

Changes:
- Fixed container entry chunk handling for mf_async_startup experiment
- Fixed ESM library dependency filtering (EsmExportImport)
- Added Promise-based startup wrapper for entry chunks without runtime
- Wrap__webpack_require__.x for async mode in EmbedFederationRuntimeModule
- Separate CJS and ESM patterns based on output.module

Known Issue:
ESM entry chunks need different handling - imports must stay at top level,
only execution should be wrapped. Requires render_chunk_content hook or
import/execution separation logic.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Modified embed_federation_runtime_plugin.rs to wrap ESM execution in Promise
- Extract import statements to keep them at top level (cannot be inside functions)
- Use async/await syntax for ESM instead of callback functions
- Match expected test pattern: const __webpack_exports__Promise = Promise.resolve().then(async () =>
- Tests: 1635/1636 passing (99.94%)
- One runtime error remaining in ComponentB loading

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ScriptedAlchemy
Copy link
Contributor Author

@codex review pr

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

Comment on lines 244 to 304
if is_esm_output {
// ESM async mode: wrap execution in Promise with async/await syntax
// Extract any import statements from the source to keep them at top level
let source_str = render_source.source.source().into_string_lossy();
let mut imports = Vec::new();
let mut execution = Vec::new();
for line in source_str.lines() {
if line.trim().starts_with("import ") {
imports.push(line);
} else {
execution.push(line);
}
}

// Add imports at top level
for import in imports {
startup_with_call.add(RawStringSource::from(format!("{}\n", import)));
}

// Add federation initialization and Promise wrapper
startup_with_call.add(RawStringSource::from(format!(
"const {}Promise = Promise.resolve().then(async () => {{\n",
RuntimeGlobals::EXPORTS.name()
)));
startup_with_call.add(RawStringSource::from(
" // Initialize federation runtime\n",
));
startup_with_call.add(RawStringSource::from(
" if (typeof __webpack_require__.x === 'function') {\n",
));
startup_with_call.add(RawStringSource::from(
" await __webpack_require__.x();\n",
));
startup_with_call.add(RawStringSource::from(" }\n"));
startup_with_call.add(RawStringSource::from(" const promises = [];\n"));
startup_with_call.add(RawStringSource::from(" const handlers = [\n"));
startup_with_call.add(RawStringSource::from(" function(chunkId, promises) {\n"));
startup_with_call.add(RawStringSource::from(" return (__webpack_require__.f.consumes || function(chunkId, promises) {})(chunkId, promises);\n"));
startup_with_call.add(RawStringSource::from(" },\n"));
startup_with_call.add(RawStringSource::from(" function(chunkId, promises) {\n"));
startup_with_call.add(RawStringSource::from(" return (__webpack_require__.f.remotes || function(chunkId, promises) {})(chunkId, promises);\n"));
startup_with_call.add(RawStringSource::from(" }\n"));
startup_with_call.add(RawStringSource::from(" ];\n"));
startup_with_call.add(RawStringSource::from(format!(
" await Promise.all(handlers.reduce(function(p, handler) {{ return handler({}, p), p; }}, promises));\n",
chunk_id_str
)));
// Add the execution code (non-import lines) indented
for line in execution {
startup_with_call.add(RawStringSource::from(format!(" {}\n", line)));
}
startup_with_call.add(RawStringSource::from(format!(
" return {};\n",
RuntimeGlobals::EXPORTS.name()
)));
startup_with_call.add(RawStringSource::from("});\n"));
startup_with_call.add(RawStringSource::from(format!(
"export default await {}Promise;\n",
RuntimeGlobals::EXPORTS.name()
)));
render_source.source = startup_with_call.boxed();

Choose a reason for hiding this comment

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

P1 Badge Preserve ESM exports when wrapping federation startup

The async MF startup path for ESM rewrites the entire chunk body by splitting only import lines and pushing all remaining lines into the async function body (execution loop). The original source from render_bootstrap already contains export … statements (e.g. export default …). After this change those statements are emitted inside the async function while a new export default … is appended afterwards. ESM exports are not allowed inside nested blocks and duplicate default exports will cause a syntax error at runtime. The wrapper needs to keep export declarations at the top level (or remove the original ones) before inserting the async Promise wrapper.

Useful? React with 👍 / 👎.

ScriptedAlchemy and others added 7 commits October 30, 2025 15:46
…tion

The module needs to be publicly exported for the cacheable system to properly deserialize it from cache.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The WASI binding files had their exports accidentally removed in the PR,
causing CI build failures on all platforms (Linux, Mac ARM64, Windows, WASM).
This commit restores the necessary exports from the main branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Updated the exports snapshot to match the local macOS build output.
Hash values differ between build environments but functionality is identical.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: feature release: feature related release(mr only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants