Skip to content
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.

Sign up for GitHub

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

feat: add BridgeReactPlugin to get federation instance #3234

Merged
merged 9 commits into from
Nov 20, 2024
Merged

feat: add BridgeReactPlugin to get federation instance #3234

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c7dbc4b
feat: mount bridge api to module instance
danpeen Nov 15, 2024
0a52f84
chore: merge main
danpeen Nov 15, 2024
9bff805
chore: update example to get instance from init api
danpeen Nov 15, 2024
153b377
Merge branch 'main' into feat/connect-bridge-with-runtime
ScriptedAlchemy Nov 16, 2024
d8dc59f
feat: update mf/enhance pkg to mf/runtime pkg
danpeen Nov 18, 2024
fbbdcbd
chore: megre main
danpeen Nov 18, 2024
9b01195
feat: adjust bridge runtime plugin
danpeen Nov 19, 2024
a3674d3
feat: get federation runtime instance
danpeen Nov 20, 2024
3f767c5
chore: update
danpeen Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/small-bats-leave.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@module-federation/bridge-react': patch
---

feat: mount bridge api to module instance
2 changes: 2 additions & 0 deletions apps/router-demo/router-host-2000/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useRef, useEffect, ForwardRefExoticComponent } from 'react';
import { Route, Routes, useLocation } from 'react-router-dom';
import { init, loadRemote } from '@module-federation/enhanced/runtime';
import { RetryPlugin } from '@module-federation/retry-plugin';
import BridgeReactPlugin from '@module-federation/bridge-react/plugin';
import { createRemoteComponent } from '@module-federation/bridge-react';
import Navigation from './navigation';
import Detail from './pages/Detail';
Expand All @@ -12,6 +13,7 @@ init({
name: 'federation_consumer',
remotes: [],
plugins: [
BridgeReactPlugin(),
RetryPlugin({
fetch: {
url: 'http://localhost:2008/not-exist-mf-manifest.json',
Expand Down
3 changes: 3 additions & 0 deletions apps/router-demo/router-remote1-2001/rsbuild.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export default defineConfig({
pluginReact(),
pluginModuleFederation({
name: 'remote1',
runtimePlugins: [
require.resolve('@module-federation/bridge-react/plugin'),
],
exposes: {
'./button': './src/button.tsx',
'./export-app': './src/export-App.tsx',
Expand Down
11 changes: 8 additions & 3 deletions packages/bridge/bridge-react/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
"import": "./dist/router.es.js",
"require": "./dist/router.cjs.js"
},
"./plugin": {
"types": "./dist/plugin.d.ts",
"import": "./dist/plugin.es.js",
"require": "./dist/plugin.es.js"
},
"./router-v5": {
"types": "./dist/router-v5.d.ts",
"import": "./dist/router-v5.es.js",
Expand All @@ -47,8 +52,7 @@
"@loadable/component": "^5.16.4",
"@module-federation/bridge-shared": "workspace:*",
"@module-federation/sdk": "workspace:*",
"react-error-boundary": "^4.0.13",
"@module-federation/runtime": "workspace:*"
"react-error-boundary": "^4.0.13"
},
"peerDependencies": {
"react": ">=16.9.0",
Expand All @@ -68,6 +72,7 @@
"react-router-dom": "6.22.3",
"typescript": "^5.2.2",
"vite": "^5.2.14",
"vite-plugin-dts": "^3.9.1"
"vite-plugin-dts": "^3.9.1",
"@module-federation/runtime": "workspace:*"
}
}
30 changes: 22 additions & 8 deletions packages/bridge/bridge-react/src/create.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { forwardRef } from 'react';
import type { FederationHost } from '@module-federation/runtime';
import {
ErrorBoundary,
ErrorBoundaryPropsWithComponent,
Expand All @@ -18,12 +19,17 @@ interface RemoteModule {
};
}

function createLazyRemoteComponent<T, E extends keyof T>(info: {
type LazyRemoteComponentInfo<T, E extends keyof T> = {
loader: () => Promise<T>;
loading: React.ReactNode;
fallback: ErrorBoundaryPropsWithComponent['FallbackComponent'];
export?: E;
}) {
instance?: FederationHost;
};

function createLazyRemoteComponent<T, E extends keyof T>(
info: LazyRemoteComponentInfo<T, E>,
) {
const exportName = info?.export || 'default';
return React.lazy(async () => {
LoggerInstance.log(`createRemoteComponent LazyComponent create >>>`, {
Expand Down Expand Up @@ -58,6 +64,7 @@ function createLazyRemoteComponent<T, E extends keyof T>(info: {
exportName={info.export || 'default'}
fallback={info.fallback}
ref={ref}
instance={info.instance}
{...props}
/>
);
Expand All @@ -83,12 +90,9 @@ function createLazyRemoteComponent<T, E extends keyof T>(info: {
});
}

export function createRemoteComponent<T, E extends keyof T>(info: {
loader: () => Promise<T>;
loading: React.ReactNode;
fallback: ErrorBoundaryPropsWithComponent['FallbackComponent'];
export?: E;
}) {
export function createRemoteComponent<T, E extends keyof T>(
info: LazyRemoteComponentInfo<T, E>,
) {
type ExportType = T[E] extends (...args: any) => any
? ReturnType<T[E]>
: never;
Expand All @@ -112,3 +116,13 @@ export function createRemoteComponent<T, E extends keyof T>(info: {
},
);
}

export function createRemoteComponentWithInstance<T, E extends keyof T>(
instance: FederationHost,
) {
return (info: LazyRemoteComponentInfo<T, E>) => {
return createLazyRemoteComponent({ ...info, instance });
Copy link
Contributor

Choose a reason for hiding this comment

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

The function could benefit from error handling for invalid input. Consider validating the instance parameter and adding error handling:

Suggested change
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 });
};

};
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
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 });
};
}

}

export type CreateRemoteComponent = typeof createRemoteComponent;
23 changes: 21 additions & 2 deletions packages/bridge/bridge-react/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
export { createRemoteComponent } from './create';
export { createBridgeComponent } from './provider';
import type { FederationRuntimePlugin } from '@module-federation/runtime';
import { createRemoteComponentWithInstance } from './create';
import { createBridgeComponentWithInstance } from './provider';
import type { CreateRemoteComponent } from './create';
import type { CreateBridgeComponent } from './provider';

let createRemoteComponent: CreateRemoteComponent;
let createBridgeComponent: CreateBridgeComponent;
function BridgeReactPlugin(): FederationRuntimePlugin {
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
let createRemoteComponent: CreateRemoteComponent;
let createBridgeComponent: CreateBridgeComponent;
class BridgeReactAPI {
static createRemoteComponent: CreateRemoteComponent;
static createBridgeComponent: CreateBridgeComponent;
}

return {
name: 'bridge-react-plugin',
beforeInit(args) {
// @ts-ignore
createRemoteComponent = createRemoteComponentWithInstance(args.origin);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

createBridgeComponent = createBridgeComponentWithInstance(args.origin);
return args;
},
Copy link
Contributor

Choose a reason for hiding this comment

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

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 { BridgeReactPlugin, createRemoteComponent, createBridgeComponent };
export type {
ProviderParams,
RenderFnParams,
Expand Down
2 changes: 2 additions & 0 deletions packages/bridge/bridge-react/src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { BridgeReactPlugin } from './index';
export default BridgeReactPlugin;
24 changes: 18 additions & 6 deletions packages/bridge/bridge-react/src/provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
import { ErrorBoundary } from 'react-error-boundary';
import { RouterContext } from './context';
import { LoggerInstance, atLeastReact18 } from './utils';
import { getInstance } from '@module-federation/runtime';
import type { FederationHost } from '@module-federation/runtime';

type RenderParams = RenderFnParams & {
[key: string]: unknown;
Comment on lines 14 to 15
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
type RenderParams = RenderFnParams & {
[key: string]: unknown;
type RenderParams = RenderFnParams & {
extraProps?: Record<string, unknown>;
};

For the second code block (lines 134-144):

Expand All @@ -26,13 +26,17 @@ type ProviderFnParams<T> = {
App: React.ReactElement,
id?: HTMLElement | string,
) => RootType | Promise<RootType>;
instance?: FederationHost;
};

export function createBridgeComponent<T>(bridgeInfo: ProviderFnParams<T>) {
return () => {
const rootMap = new Map<any, RootType>();
const instance = getInstance();
LoggerInstance.log(`createBridgeComponent remote instance`, instance);
const { instance } = bridgeInfo;
LoggerInstance.log(
`createBridgeComponent instance from props >>>`,
instance,
);

const RawComponent = (info: { propsInfo: T; appInfo: ProviderParams }) => {
const { appInfo, propsInfo, ...restProps } = info;
Expand Down Expand Up @@ -95,15 +99,13 @@ export function createBridgeComponent<T>(bridgeInfo: ProviderFnParams<T>) {
const renderFn = bridgeInfo?.render || ReactDOM.render;
renderFn?.(rootComponentWithErrorBoundary, info.dom);
}

instance?.bridgeHook?.lifecycle?.afterBridgeRender?.emit(info) || {};
},

async destroy(info: DestroyParams) {
LoggerInstance.log(`createBridgeComponent destroy Info`, {
dom: info.dom,
});

instance?.bridgeHook?.lifecycle?.beforeBridgeDestroy?.emit(info);

// call destroy function
Expand All @@ -115,7 +117,9 @@ export function createBridgeComponent<T>(bridgeInfo: ProviderFnParams<T>) {
ReactDOM.unmountComponentAtNode(info.dom);
}

instance?.bridgeHook?.lifecycle?.afterBridgeDestroy?.emit(info);
(instance as any)?.bridgeHook?.lifecycle?.afterBridgeDestroy?.emit(
info,
);
},
rawComponent: bridgeInfo.rootComponent,
__BRIDGE_FN__: (_args: T) => {},
Expand All @@ -130,3 +134,11 @@ export function ShadowRoot(info: { children: () => JSX.Element }) {

return <div ref={domRef}>{root && <info.children />}</div>;
}

export function createBridgeComponentWithInstance<T>(instance: FederationHost) {
return (bridgeInfo: ProviderFnParams<T>) => {
return createBridgeComponent({ ...bridgeInfo, instance });
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
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.


export type CreateBridgeComponent = typeof createBridgeComponent;
19 changes: 9 additions & 10 deletions packages/bridge/bridge-react/src/remote/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { ProviderParams } from '@module-federation/bridge-shared';
import { dispatchPopstateEnv } from '@module-federation/bridge-shared';
import { ErrorBoundaryPropsWithComponent } from 'react-error-boundary';
import { LoggerInstance, pathJoin, getRootDomDefaultClassName } from '../utils';
import { getInstance } from '@module-federation/runtime';
import type { FederationHost } from '@module-federation/runtime';

declare const __APP_VERSION__: string;
export interface RenderFnParams extends ProviderParams {
Expand All @@ -34,6 +34,7 @@ interface RemoteAppParams {
providerInfo: NonNullable<RemoteModule['provider']>;
exportName: string | number | symbol;
fallback: ErrorBoundaryPropsWithComponent['FallbackComponent'];
instance: FederationHost;
}

const RemoteAppWrapper = forwardRef(function (
Expand All @@ -50,6 +51,7 @@ const RemoteAppWrapper = forwardRef(function (
className,
style,
fallback,
instance,
...resProps
} = props;

Expand All @@ -60,8 +62,7 @@ const RemoteAppWrapper = forwardRef(function (

const renderDom: React.MutableRefObject<HTMLElement | null> = useRef(null);
const providerInfoRef = useRef<any>(null);
const hostInstance = getInstance();
LoggerInstance.log(`RemoteAppWrapper hostInstance >>>`, hostInstance);
LoggerInstance.log(`RemoteAppWrapper instance from props >>>`, instance);

useEffect(() => {
const renderTimeout = setTimeout(() => {
Expand All @@ -84,18 +85,16 @@ const RemoteAppWrapper = forwardRef(function (

LoggerInstance.log(
`createRemoteComponent LazyComponent hostInstance >>>`,
hostInstance,
instance,
);
const beforeBridgeRenderRes =
hostInstance?.bridgeHook?.lifecycle?.beforeBridgeRender?.emit(
instance?.bridgeHook?.lifecycle?.beforeBridgeRender?.emit(
renderProps,
) || {};
// @ts-ignore
renderProps = { ...renderProps, ...beforeBridgeRenderRes.extraProps };
providerReturn.render(renderProps);
hostInstance?.bridgeHook?.lifecycle?.afterBridgeRender?.emit(
renderProps,
);
instance?.bridgeHook?.lifecycle?.afterBridgeRender?.emit(renderProps);
});

return () => {
Expand All @@ -107,7 +106,7 @@ const RemoteAppWrapper = forwardRef(function (
{ moduleName, basename, dom: renderDom.current },
);

hostInstance?.bridgeHook?.lifecycle?.beforeBridgeDestroy?.emit({
instance?.bridgeHook?.lifecycle?.beforeBridgeDestroy?.emit({
moduleName,
dom: renderDom.current,
basename,
Expand All @@ -121,7 +120,7 @@ const RemoteAppWrapper = forwardRef(function (
dom: renderDom.current,
});

hostInstance?.bridgeHook?.lifecycle?.afterBridgeDestroy?.emit({
instance?.bridgeHook?.lifecycle?.afterBridgeDestroy?.emit({
moduleName,
dom: renderDom.current,
basename,
Expand Down
2 changes: 1 addition & 1 deletion packages/bridge/bridge-react/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default defineConfig({
lib: {
entry: {
index: path.resolve(__dirname, 'src/index.ts'),
plugin: path.resolve(__dirname, 'src/plugin.ts'),
router: path.resolve(__dirname, 'src/router.tsx'),
'router-v5': path.resolve(__dirname, 'src/router-v5.tsx'),
'router-v6': path.resolve(__dirname, 'src/router-v6.tsx'),
Expand All @@ -36,7 +37,6 @@ export default defineConfig({
'react-router-dom/',
'react-router-dom/index.js',
'react-router-dom/dist/index.js',
'@module-federation/runtime',
],
plugins: [
{
Expand Down
8 changes: 3 additions & 5 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading