-
- Notifications
You must be signed in to change notification settings - Fork 360
feat(bridge-react): add rerender option to createBridgeComponent (#4171) #4172
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
base: main
Are you sure you want to change the base?
Conversation
path.join is not meant for URLs. Code does not work in Node > 22.11 and produces an Invalid URL. Fixing by setting the origin as the URL base.
Fixes CI format check failure by adding required trailing comma.
- Add rerender option to ProviderFnParams interface for custom rerender handling - Update bridge-base implementation to support custom rerender logic - Add component state tracking to detect rerenders vs initial renders - Preserve component state when shouldRecreate is false - Maintain backward compatibility for existing code - Add comprehensive test suite for rerender functionality Fixes #4171
🦋 Changeset detectedLatest commit: e730e0d The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
💡 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".
packages/bridge/bridge-react/src/provider/versions/bridge-base.tsx Outdated Show resolved Hide resolved
| const url = new URL( | ||
| path.join(origin, typescriptFolderName, file), | ||
| path.join(typescriptFolderName, file), | ||
| origin, | ||
| ).toString(); |
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.
URL builder loses remote pathname in type download
The URL construction now uses new URL(path.join(typescriptFolderName, file), origin) which resolves the relative path against the origin by replacing its pathname. If a remote serves types from a subpath (e.g. https://host/app-a), this produces https://host/federated-types/..., dropping the app-a segment and causing the subsequent download to look in the wrong location. The previous code preserved the full origin path. Include the origin pathname in the joined path so remotes hosted under subpaths still download correctly.
Useful? React with 👍 / 👎.
- Fix shouldRecreate: true to actually unmount and recreate the root - Implement proper root recreation with fresh React root instance - Add comprehensive test to verify recreation behavior - Ensure state is truly reset when shouldRecreate is true - Maintain proper cleanup of old roots before creating new ones - Add changeset for version bumping - Remove test-implementation.js file (tests are in proper package location) This addresses the issue where shouldRecreate: true was not actually recreating the component and resetting state as promised in the API.
- Fix shouldRecreate: true to actually unmount and recreate the root - Implement proper root recreation with fresh React root instance - Add comprehensive test to verify recreation behavior - Ensure state is truly reset when shouldRecreate is true - Maintain proper cleanup of old roots before creating new ones This addresses the issue where shouldRecreate: true was not actually recreating the component and resetting state as promised in the API.
| @codex review pr |
- Revert packages/typescript/src/plugins/FederatedTypesPlugin.ts to main branch version - This file was not intended to be part of the bridge-react rerender functionality PR - Keep PR focused only on bridge-react changes for issue #4171
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.
💡 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".
| } else { | ||
| // Custom rerender function requested recreation - unmount and recreate | ||
| LoggerInstance.debug( | ||
| `createBridgeComponent recreating component due to shouldRecreate: true >>>`, | ||
| info, | ||
| ); | ||
| | ||
| // Unmount the existing root to reset all state | ||
| if (root && 'unmount' in root) { | ||
| root.unmount(); | ||
| LoggerInstance.debug( | ||
| `createBridgeComponent unmounted existing root >>>`, | ||
| info, | ||
| ); | ||
| } | ||
| |
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.
Emit destroy hooks when recreating root
When rerender requests recreation, the code unmounts the existing root and immediately creates a fresh one but never emits beforeBridgeDestroy/afterBridgeDestroy. In the normal destroy path those hooks run and allow listeners to tear down resources. For components that subscribe to the lifecycle hooks (e.g. RemoteAppWrapper and plugin consumers), forcing recreation will leak listeners and leave caches stale because they never see the destroy event. Consider invoking the same lifecycle hooks around this manual unmount or delegating to the existing destroy() logic before creating the new root.
Useful? React with 👍 / 👎.
| @codex address all pr comments. |
| @codex review pr |
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.
💡 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".
| // Create a new BridgeWrapper with updated props for rerender | ||
| const { | ||
| moduleName: updatedModuleName, | ||
| basename: updatedBasename, | ||
| memoryRoute: updatedMemoryRoute, | ||
| fallback: updatedFallback, | ||
| ...updatedPropsInfo | ||
| } = info; | ||
| | ||
| const UpdatedBridgeWrapper = ({ | ||
| basename, | ||
| }: { | ||
| basename?: string; | ||
| }) => ( | ||
| <ErrorBoundary | ||
| FallbackComponent={ | ||
| updatedFallback as React.ComponentType<FallbackProps> | ||
| } | ||
| > | ||
| <RawComponent | ||
| appInfo={{ | ||
| moduleName: updatedModuleName, | ||
| basename, | ||
| memoryRoute: updatedMemoryRoute, | ||
| }} | ||
| propsInfo={ | ||
| { | ||
| ...updatedPropsInfo, | ||
| basename, | ||
| ...(beforeBridgeRenderRes as any)?.extraProps, | ||
| } as T | ||
| } | ||
| /> | ||
| </ErrorBoundary> | ||
| ); | ||
| | ||
| const updatedRootComponentWithErrorBoundary = ( | ||
| <UpdatedBridgeWrapper basename={updatedBasename} /> | ||
| ); |
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.
Preserve component identity during rerender
When rerender returns { shouldRecreate: false }, the code still defines a new UpdatedBridgeWrapper function and element for every call before invoking root.render. Because a React component’s identity is the function reference, creating a new arrow function here causes React to unmount the previous tree and mount a new one on each host rerender, so internal state is still lost even though the option is meant to avoid recreation. The stored componentStateMap/propsStateMap are never reused to render the same element, so the new rerender API does not actually preserve state. A single stable wrapper component (or reusing the stored element) is needed to prevent unmounting.
Useful? React with 👍 / 👎.
| Summary
Testing
|
Summary
This PR implements the rerender functionality requested in issue #4171 to prevent remote apps from being recreated when host components rerender.
Problem Solved
When a host component rerenders and passes new props to a remote app component created with
createRemoteAppComponent, the entire remote app gets recreated instead of just updating with the new props. This causes:Solution
Added a
rerenderoption tocreateBridgeComponentthat allows developers to control how their remote apps handle rerenders:Changes Made
1. Type Definitions (
types.ts)rerender?: (props: RenderParams) => { shouldRecreate?: boolean } | voidtoProviderFnParams<T>2. Bridge Implementation (
bridge-base.tsx)componentStateMapandpropsStateMapshouldRecreateflag to control recreation behavior3. Test Suite (
rerender-issue.spec.tsx)Usage Examples
Prevent Recreation (Preserve State)
Conditional Recreation
Custom Logic with Default Behavior
Benefits
Testing
The implementation includes comprehensive tests that verify:
Fixes #4171