Skip to content

feat(runtime): provide a root #3740

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 apps/runtime-demo/3005-runtime-host/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Link, Routes, Route, BrowserRouter } from 'react-router-dom';
import Root from './Root';
import Remote1 from './Remote1';
import Remote2 from './Remote2';
import Remote3 from './Remote3';

const App = () => (
<BrowserRouter>
Expand All @@ -18,11 +19,15 @@ const App = () => (
<li>
<Link to={'/remote2'}>remote2</Link>
</li>
<li>
<Link to={'/remote3'}>remote3</Link>
</li>
</ul>
<Routes>
<Route path="/" element={<Root />} />
<Route path="/remote1" element={<Remote1 />} />
<Route path="/remote2" element={<Remote2 />} />
<Route path="/remote3" element={<Remote3 />} />
Copy link
Author

Choose a reason for hiding this comment

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

Behavior is odd... Loading the page directly attaches the CSS correctly, navigating to it through a link doesn't because css assets is missing in the manifest... It's almost like the built code caches something different than the hardened runtime which uses mf-manifest.json?

</Routes>
</BrowserRouter>
);
Expand Down
28 changes: 28 additions & 0 deletions apps/runtime-demo/3005-runtime-host/src/Remote3.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React, { Suspense, lazy } from 'react';
import { createRoot } from 'react-dom/client';
import { loadRemote } from '@module-federation/enhanced/runtime';

class CustomElement extends HTMLElement {
constructor() {
super();
this.attachShadow({ mode: 'open' });
}
async connectedCallback() {
if (!this.shadowRoot) return;

const module = await loadRemote('dynamic-remote/ButtonOldAnt', {
//@ts-ignore
root: this.shadowRoot,
});
//@ts-ignore
createRoot(this.shadowRoot).render(React.createElement(module.default));
}
}

customElements.define('custom-element', CustomElement);

function DynamicRemoteButton() {
return React.createElement('custom-element');
}

export default DynamicRemoteButton;
4 changes: 2 additions & 2 deletions packages/runtime-core/src/plugins/snapshot/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function snapshotPlugin(): FederationRuntimePlugin {
return {
name: 'snapshot-plugin',
async afterResolve(args) {
const { remote, pkgNameOrAlias, expose, origin, remoteInfo } = args;
const { remote, pkgNameOrAlias, expose, origin, remoteInfo, root } = args;

if (!isRemoteInfoWithEntry(remote) || !isPureRemoteEntry(remote)) {
const { remoteSnapshot, globalSnapshot } =
Expand Down Expand Up @@ -73,7 +73,7 @@ export function snapshotPlugin(): FederationRuntimePlugin {
);

if (assets) {
preloadAssets(remoteInfo, origin, assets, false);
preloadAssets(remoteInfo, origin, assets, false, root);
}

return {
Expand Down
10 changes: 8 additions & 2 deletions packages/runtime-core/src/remote/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface LoadRemoteMatch {
origin: FederationHost;
remoteInfo: RemoteInfo;
remoteSnapshot?: ModuleInfo;
root?: HTMLElement;
}

export class RemoteHandler {
Expand Down Expand Up @@ -197,7 +198,7 @@ export class RemoteHandler {
// eslint-disable-next-line @typescript-eslint/member-ordering
async loadRemote<T>(
id: string,
options?: { loadFactory?: boolean; from: CallFrom },
options?: { loadFactory?: boolean; from: CallFrom; root?: HTMLElement },
): Promise<T | null> {
const { host } = this;
try {
Expand All @@ -214,6 +215,7 @@ export class RemoteHandler {
const { module, moduleOptions, remoteMatchInfo } =
await this.getRemoteModuleAndOptions({
id,
root: options?.root,
});
const {
pkgNameOrAlias,
Expand Down Expand Up @@ -314,7 +316,10 @@ export class RemoteHandler {
});
}

async getRemoteModuleAndOptions(options: { id: string }): Promise<{
async getRemoteModuleAndOptions(options: {
id: string;
root?: HTMLElement;
}): Promise<{
module: Module;
moduleOptions: ModuleOptions;
remoteMatchInfo: LoadRemoteMatch;
Expand Down Expand Up @@ -371,6 +376,7 @@ export class RemoteHandler {
options: host.options,
origin: host,
remoteInfo,
root: options.root,
});

const { remote, expose } = matchInfo;
Expand Down
8 changes: 6 additions & 2 deletions packages/runtime-core/src/utils/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function preloadAssets(
assets: PreloadAssets,
// It is used to distinguish preload from load remote parallel loading
useLinkPreload = true,
root: HTMLElement = document.head,
Copy link
Author

Choose a reason for hiding this comment

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

A fatal flaw with the current approach is that when you load something (or preload something) it gets cached by id in a global. Subsequent calls to retrieve assets no longer returns those assets. So ultimately, Federation only supports a single root per global. 😞 To fix this I think we'd need to make sure that this code:

https://github.com/iammerrick/module-federation-core/blob/7d26e1325c4520534ee976d131cc6a8e9391c8b5/packages/runtime-core/src/plugins/generate-preload-assets.ts#L248

Somehow respects caches on whether it's been loaded for a specific root. Not assuming a global loading approach.

): void {
const { cssAssets, jsAssetsWithoutEntry, entryAssets } = assets;

Expand Down Expand Up @@ -99,6 +100,7 @@ export function preloadAssets(
};
cssAssets.forEach((cssUrl) => {
const { link: cssEl, needAttach } = createLink({
root,
url: cssUrl,
cb: () => {
// noop
Expand All @@ -116,7 +118,7 @@ export function preloadAssets(
},
});

needAttach && document.head.appendChild(cssEl);
needAttach && root.appendChild(cssEl);
});
} else {
const defaultAttrs = {
Expand All @@ -125,6 +127,7 @@ export function preloadAssets(
};
cssAssets.forEach((cssUrl) => {
const { link: cssEl, needAttach } = createLink({
root,
url: cssUrl,
cb: () => {
// noop
Expand All @@ -143,7 +146,7 @@ export function preloadAssets(
needDeleteLink: false,
});

needAttach && document.head.appendChild(cssEl);
needAttach && root.appendChild(cssEl);
});
}

Expand All @@ -154,6 +157,7 @@ export function preloadAssets(
};
jsAssetsWithoutEntry.forEach((jsUrl) => {
const { link: linkEl, needAttach } = createLink({
root: document.head,
url: jsUrl,
cb: () => {
// noop
Expand Down
3 changes: 2 additions & 1 deletion packages/sdk/src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export function createScript(info: {
}

export function createLink(info: {
root: HTMLElement;
url: string;
cb?: (value: void | PromiseLike<void>) => void;
onErrorCallback?: (error: Error) => void;
Expand All @@ -151,7 +152,7 @@ export function createLink(info: {
// Retrieve the existing script element by its src attribute
let link: HTMLLinkElement | null = null;
let needAttach = true;
const links = document.getElementsByTagName('link');
const links = info.root.querySelectorAll('link');
for (let i = 0; i < links.length; i++) {
const l = links[i];
const linkHref = l.getAttribute('href');
Expand Down