feat(bridge-react): add rerender option to createBridgeComponent (#4171) #4172
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: 2a4045b 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 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
- 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.
💡 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".
|
@codex address all pr comments. |
|
@codex review pr |
There was a problem hiding this comment.
💡 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
|
Summary
Testing
|
|
Could you tell me please: how will it work with custom If I write something like this: export default createBridgeComponent({
rootComponent: App,
// Custom render function
render: (App, container) => {
const root = createRoot(container as HTMLElement, {
identifierPrefix: 'my-app-'
});
root.render(App);
return root;
},
});What |
|
Hey folks! 👋 @ScriptedAlchemy, Could be verified with a small snipet in mentioned test code: function RemoteApp({ props }: { props?: { count: number } }) {
const instanceId = useRef(++instanceCounter);
React.useEffect(() => {
console.log('MOUNT');
return () => {
console.log('UNMOUNT');
};
}, []);
....
}The core reason of losing state is laying in react reconcilation logic: component with differ type will fully replace the original one, no matter of props. BridgeWrapper and UpdatedBridgeWrapper will get different type each In case of "typical" component code the eslint will help to catch such things with the error: It's valid for bridge Potential fix: Closing thoughts: |
- use stable element types (no nested component defs) - create root only on first render; reuse for updates - emit before/after destroy hooks when recreating - honor custom render: skip re-creating root on updates
|
Addressed review feedback:
Relevant changes:
All bridge-react tests pass locally. Happy to adjust further if you want different naming or hook placement. |
|
@ScriptedAlchemy the initial problem solved by latest commits, thx a lot! :) I still wonder what is the real use case under the recreation logic? When it could be useful? |
|
Thanks all for the thoughtful feedback — I’ve addressed the concerns and expanded tests accordingly.
What changed since the original submission
Relevant files
Happy to tweak naming or placement if you’d prefer a different structure. Thanks again! 🙌 |
|
@codex review pr |
There was a problem hiding this comment.
💡 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".
… root has no render() Add tests to cover fallback behavior and ensure UI updates + custom render is invoked again.
|
Thanks for the catch — addressed.
Files:
If you prefer that we also emit destroy hooks in this fallback path, happy to wire that in — right now we keep it as a pure update for backward-compat parity. |
Yeah, the main problem was the UNMOUNT/MOUNT problem. So I suggested the At first I tried to implement it by myself (without Here is my implementation: // remote
export default function createProvider() {
const rootMap = new Map();
const listenersMap = new Map();
const propsMap = new Map();
const Renderer = ({ dom }) => {
const [_, setState] = React.useState(0)
React.useEffect(() => {
listenersMap.set(dom, () => {
setState(s => s + 1)
})
return () => listenersMap.delete(dom)
}, [])
const props = propsMap.get(dom)
return <App {...props} />
}
return {
// Render application to specified DOM node
render(props, dom) {
const root = createRoot(dom, { identifierPrefix: 'bridge' });
rootMap.set(dom, root);
propsMap.set(dom, props)
root.render(
<Renderer dom={dom} />
);
},
rerender(props, dom) {
propsMap.set(dom, props)
const listener = listenersMap.get(dom);
if (listener) {
listener()
}
},
// Clean up resources
destroy(info, dom) {
const root = rootMap.get(dom);
root?.unmount();
rootMap.delete(dom);
listenersMap.delete(dom)
propsMap.delete(dom)
},
}
}// host
const RootApp = React.lazy(async () => {
// Load remote module
const module = await mf.loadRemote('...');
const provider = module.default;
return {
default: (props) => {
const containerRef = React.useRef(null);
const providerRef = React.useRef(null);
React.useEffect(() => {
if (containerRef.current) {
// Create provider instance
const instance = provider();
providerRef.current = instance;
// Render remote application
instance.render(props, containerRef.current);
}
// Cleanup function
return () => {
if (providerRef.current && containerRef.current) {
providerRef.current.destroy({}, containerRef.current);
providerRef.current = undefined
}
};
}, []);
React.useMemo(() => {
if (providerRef.current) {
const instance = providerRef.current
// Rerender remote application
instance.rerender(props, containerRef.current);
}
}, [props])
return <div ref={containerRef} />;
}
};
});So I thought about As I can see now, So |
|
Honestly, I don't see cases where I will use |
|
@crutch12 that's what I was thinking too: if I want to recrerate the tree I would use the react technique instead, like drill the unqiue key prop value. But as I understand @ScriptedAlchemy has a strong opinion to implement Just to reiterate: |
Summary
This PR implements stable rerender behavior in the bridge so remote apps are NOT recreated on host prop changes. It adds an optional
rerenderhook for conditional recreation, and ensures customrender(App, container)implementations interop correctly.Fixes #4171
Problem
Previously, each host rerender rebuilt different wrapper component types inside the bridge. React treated these as new elements and unmounted/remounted the remote tree, causing:
render()integrationsWhat Changed
root.render().rerender(info)may return{ shouldRecreate: true }to explicitly recreate a root; otherwise props update in place.render(App, dom)is called on initial mount only; the returned root is reused for updates.beforeBridgeDestroyandafterBridgeDestroyfire during explicit recreation.API
rerenderto always preserve state on prop changes.rerenderonly when you truly need to rebuild (e.g., version swap, incompatible context change).Tests
Added/extended tests in
packages/bridge/bridge-react/__tests__/rerender-issue.spec.tsx:rerender(back compat) and withrerenderreturning voidrender(App, container)preserves state on updates and is invoked again only on recreationbeforeBridgeRendercan injectextraPropsinto child propsRouter test utility no longer hard-depends on
jsdom; it falls back to Jest's jsdom environment for portability.Backward Compatibility
rerenderhook is opt-in and defaults to preserving state.Implementation Notes
packages/bridge/bridge-react/src/provider/versions/bridge-base.tsx.packages/bridge/bridge-react/__tests__/rerender-issue.spec.tsx.Maintenance