feat: add BridgeReactPlugin to get federation instance #3234
Conversation
🦋 Changeset detectedLatest commit: 3f767c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 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 site configuration. |
![squadronai[bot]](https://avatars.githubusercontent.com/in/901096?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This pull request introduces a new feature to the Bridge React Plugin that allows for better integration and management of the bridge API within the module instance. The key changes include:
- A new
createLazyRemoteComponentfunction that creates a React.lazy component to dynamically import remote modules and handle loading and error states. - The
createRemoteComponentfunction has been updated to use theLazyRemoteComponentInfotype, simplifying the function signature. - The introduction of a new
BridgeReactPluginthat stores theFederationHostinstance in a globalfederationRuntimeobject, making it accessible to other parts of the application. - The
BridgeReactPluginnow includes afederationRuntimeproperty to access the federation instance, enabling better integration with the bridge API. - The
remote/index.tsxfile has been updated to mount the bridge API to the module instance, including lifecycle hooks forbeforeBridgeRender,afterBridgeRender,beforeBridgeDestroy, andafterBridgeDestroyto handle the rendering and destruction of the remote component.
These changes aim to improve the overall functionality and integration of the bridge API within the module instance, providing a more seamless and efficient development experience.
File Summaries
| File | Summary |
|---|---|
| packages/bridge/bridge-react/src/create.tsx | The code changes introduce a new function createLazyRemoteComponent that takes in an object with properties loader, loading, fallback, and an optional export. This function creates a React.lazy component that dynamically imports the remote module and handles loading and error states. Additionally, the createRemoteComponent function is updated to use the LazyRemoteComponentInfo type, simplifying the function signature. |
| packages/bridge/bridge-react/src/plugin.ts | This code introduces a new plugin called BridgeReactPlugin that is designed to work with the @module-federation/runtime library. The plugin's primary purpose is to store the FederationHost instance in a global federationRuntime object, which can be accessed by other parts of the application. |
| packages/bridge/bridge-react/src/provider.tsx | The code changes introduce a new feature to the BridgeReactPlugin by adding a federationRuntime property to access the federation instance. This allows for better integration and management of the bridge API within the module instance. |
| packages/bridge/bridge-react/src/remote/index.tsx | The code changes introduce a new feature to mount the bridge API to the module instance. The key modifications include replacing the getInstance() function with the federationRuntime.instance to access the federation instance, and adding lifecycle hooks for beforeBridgeRender, afterBridgeRender, beforeBridgeDestroy, and afterBridgeDestroy to handle the rendering and destruction of the remote component. |
danpeen
changed the title
There was a problem hiding this comment.
Incremental Review
Comments posted: 8
Configuration
Squadron Mode: essential
Commits Reviewed
9bff805fe24953c5368a4eecce08fcea3f0bd9c0...9b011959e29a4afbdb1ac4d6b8429ab870ae9364
Files Reviewed
- packages/bridge/bridge-react/src/create.tsx
- packages/bridge/bridge-react/src/plugin.ts
- packages/bridge/bridge-react/src/provider.tsx
- packages/bridge/bridge-react/src/remote/index.tsx
- packages/bridge/bridge-react/src/index.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/small-bats-leave.md
- apps/router-demo/router-host-2000/src/App.tsx
- apps/router-demo/router-remote1-2001/rsbuild.config.ts
- packages/bridge/bridge-react/package.json
- packages/bridge/bridge-react/vite.config.ts
- pnpm-lock.yaml
| export function createRemoteComponentWithInstance<T, E extends keyof T>( | ||
| instance: FederationHost, | ||
| ) { | ||
| return (info: LazyRemoteComponentInfo<T, E>) => { | ||
| return createLazyRemoteComponent({ ...info, instance }); | ||
| }; |
There was a problem hiding this comment.
The function name createRemoteComponentWithInstance could be more specific about its purpose. Consider adding TypeScript parameter documentation to explain what T and E represent, and what the function returns. Here's a suggested improvement:
| export function createRemoteComponentWithInstance<T, E extends keyof T>( | |
| instance: FederationHost, | |
| ) { | |
| return (info: LazyRemoteComponentInfo<T, E>) => { | |
| return createLazyRemoteComponent({ ...info, instance }); | |
| }; | |
| /** | |
| * Creates a remote component factory using a specific Federation Host instance | |
| * @param instance - The Federation Host instance to use for component loading | |
| * @returns A function that creates lazy-loaded remote components | |
| * @template T - The module's exports type | |
| * @template E - The specific export key from the module | |
| */ | |
| export function createRemoteComponentWithInstance<T, E extends keyof T>( | |
| instance: FederationHost, | |
| ) { | |
| return (info: LazyRemoteComponentInfo<T, E>) => { | |
| return createLazyRemoteComponent({ ...info, instance }); | |
| }; | |
| } |
| return (info: LazyRemoteComponentInfo<T, E>) => { | ||
| return createLazyRemoteComponent({ ...info, instance }); |
There was a problem hiding this comment.
The function could benefit from error handling for invalid input. Consider validating the instance parameter and adding error handling:
| return (info: LazyRemoteComponentInfo<T, E>) => { | |
| return createLazyRemoteComponent({ ...info, instance }); | |
| return (info: LazyRemoteComponentInfo<T, E>) => { | |
| if (!instance) { | |
| throw new Error('Federation Host instance is required'); | |
| } | |
| return createLazyRemoteComponent({ ...info, instance }); | |
| }; |
| type RenderParams = RenderFnParams & { | ||
| [key: string]: unknown; |
There was a problem hiding this comment.
The RenderParams type extends RenderFnParams but allows arbitrary additional properties through an index signature. This could lead to type safety issues. Consider explicitly defining expected properties or using a more specific type:
| type RenderParams = RenderFnParams & { | |
| [key: string]: unknown; | |
| type RenderParams = RenderFnParams & { | |
| extraProps?: Record<string, unknown>; | |
| }; |
For the second code block (lines 134-144):
| export function createBridgeComponentWithInstance<T>(instance: FederationHost) { | ||
| return (bridgeInfo: ProviderFnParams<T>) => { | ||
| return createBridgeComponent({ ...bridgeInfo, instance }); | ||
| }; | ||
| } |
There was a problem hiding this comment.
The createBridgeComponentWithInstance function could benefit from memoization since it creates a new function on each call. Consider using React.useCallback or memoizing the returned function:
| export function createBridgeComponentWithInstance<T>(instance: FederationHost) { | |
| return (bridgeInfo: ProviderFnParams<T>) => { | |
| return createBridgeComponent({ ...bridgeInfo, instance }); | |
| }; | |
| } | |
| export function createBridgeComponentWithInstance<T>(instance: FederationHost) { | |
| return React.useCallback( | |
| (bridgeInfo: ProviderFnParams<T>) => createBridgeComponent({ ...bridgeInfo, instance }), | |
| [instance] | |
| ); | |
| } |
These suggestions focus on improving type safety and performance while maintaining the code's functionality within their respective commit ranges.
| return { | ||
| name: 'bridge-react-plugin', | ||
| beforeInit(args) { | ||
| // @ts-ignore |
There was a problem hiding this comment.
Remove the @ts-ignore comment and properly type the args.origin. This improves type safety and maintainability. If origin is expected on args, consider extending the type definition of FederationRuntimePlugin's beforeInit parameters.
| let createRemoteComponent: CreateRemoteComponent; | ||
| let createBridgeComponent: CreateBridgeComponent; |
There was a problem hiding this comment.
These variables are mutable globals which could lead to race conditions and make testing difficult. Consider using a class or module pattern instead. Here's a suggestion:
| let createRemoteComponent: CreateRemoteComponent; | |
| let createBridgeComponent: CreateBridgeComponent; | |
| class BridgeReactAPI { | |
| static createRemoteComponent: CreateRemoteComponent; | |
| static createBridgeComponent: CreateBridgeComponent; | |
| } |
| // @ts-ignore | ||
| createRemoteComponent = createRemoteComponentWithInstance(args.origin); | ||
| createBridgeComponent = createBridgeComponentWithInstance(args.origin); | ||
| return args; |
There was a problem hiding this comment.
The beforeInit function returns args without any transformation. If no modification is needed, consider documenting why this pass-through is necessary, or remove the return statement if it's not required by the plugin interface.
| export type { | ||
| ProviderParams, | ||
| RenderFnParams, |
There was a problem hiding this comment.
The type exports appear to be incomplete, ending with a comma suggesting there are more types to export. Ensure all necessary types are properly exported for API completeness and documentation.
danpeen
changed the title
Description
feat: mount bridge api to module instance
Related Issue
Types of changes
Checklist