From dfc8162db7e66ddabe5c3e9ee9de7bb8e1a12d86 Mon Sep 17 00:00:00 2001 From: Nicholas Cunningham Date: Mon, 26 May 2025 09:25:32 -0600 Subject: [PATCH] fix(module-federation): enhance remote entry handling with query parameters in paths (#30615) ## Current Behavior In Module Federation apps, when remotes are defined using URLs that include query string or hash fragments (e.g. for cache busting), those params are not preserved after the application is built. ## Expected Behavior This PR ensures that query strings and hash fragments are preserved when resolving or generating remote URLs. ## Related Issue(s) Fixes #30602 --- .../src/module-federation/core.rspack.test.ts | 59 ++++++++++++++++++ .../module-federation/core.webpack.test.ts | 60 +++++++++++++++++++ packages/angular/mf/mf.ts | 18 ++++-- .../module-federation/src/utils/remotes.ts | 58 +++++++++--------- .../rspack/with-module-federation.ts | 1 - packages/react/mf/dynamic-federation.ts | 18 ++++-- 6 files changed, 173 insertions(+), 41 deletions(-) diff --git a/e2e/react/src/module-federation/core.rspack.test.ts b/e2e/react/src/module-federation/core.rspack.test.ts index b58c2a906b..be41596882 100644 --- a/e2e/react/src/module-federation/core.rspack.test.ts +++ b/e2e/react/src/module-federation/core.rspack.test.ts @@ -5,6 +5,7 @@ import { killPorts, killProcessAndPorts, newProject, + readJson, runCLIAsync, runCommandUntil, runE2ETests, @@ -227,5 +228,63 @@ describe('React Rspack Module Federation', () => { remotePort ); }, 500_000); + it('should preserve remotes with query params in the path', async () => { + const shell = uniq('shell'); + const remote1 = uniq('remote1'); + + runCLI( + `generate @nx/react:host apps/${shell} --name=${shell} --remotes=${remote1} --bundler=rspack --e2eTestRunner=none --style=css --no-interactive --skipFormat` + ); + + // Update the remote entry to include query params + updateFile(`apps/${shell}/module-federation.config.ts`, (content) => + content.replace( + `"${remote1}"`, + `['${remote1}', 'http://localhost:4201/remoteEntry.js?param=value']` + ) + ); + + runCLI(`run ${shell}:build:production`); + + // Check the artifact in dist for the remote + const manifestJson = readJson(`dist/apps/${shell}/mf-manifest.json`); + const remoteEntry = manifestJson.remotes[0]; // There should be only one remote + + expect(remoteEntry).toBeDefined(); + expect(remoteEntry.entry).toContain( + 'http://localhost:4201/remoteEntry.js?param=value' + ); + expect(manifestJson.remotes).toMatchInlineSnapshot(` + [ + { + "alias": "${remote1}", + "entry": "http://localhost:4201/remoteEntry.js?param=value", + "federationContainerName": "${remote1}", + "moduleName": "Module", + }, + ] + `); + + // Update the remote entry to include new query params without remoteEntry.js + updateFile(`apps/${shell}/module-federation.config.ts`, (content) => + content.replace( + 'http://localhost:4201/remoteEntry.js?param=value', + 'http://localhost:4201?param=newValue' + ) + ); + + runCLI(`run ${shell}:build:production`); + + // Check the artifact in dist for the remote + const manifestJsonUpdated = readJson( + `dist/apps/${shell}/mf-manifest.json` + ); + const remoteEntryUpdated = manifestJsonUpdated.remotes[0]; // There should be only one remote + + expect(remoteEntryUpdated).toBeDefined(); + expect(remoteEntryUpdated.entry).toContain( + 'http://localhost:4201/remoteEntry.js?param=newValue' + ); + }); }); }); diff --git a/e2e/react/src/module-federation/core.webpack.test.ts b/e2e/react/src/module-federation/core.webpack.test.ts index d3718cf23f..416faba666 100644 --- a/e2e/react/src/module-federation/core.webpack.test.ts +++ b/e2e/react/src/module-federation/core.webpack.test.ts @@ -5,6 +5,7 @@ import { killPorts, killProcessAndPorts, newProject, + readJson, runCLIAsync, runCommandUntil, runE2ETests, @@ -187,6 +188,65 @@ describe('React Module Federation', () => { } }, 500_000); + it('should preserve remotes with query params in the path', async () => { + const shell = uniq('shell'); + const remote1 = uniq('remote1'); + + runCLI( + `generate @nx/react:host apps/${shell} --name=${shell} --remotes=${remote1} --bundler=webpack --e2eTestRunner=none --style=css --no-interactive --skipFormat` + ); + + // Update the remote entry to include query params at the end with remoteEntry in path + updateFile(`apps/${shell}/webpack.config.prod.ts`, (content) => + content.replace( + `'http://localhost:4201/'`, + `'http://localhost:4201/remoteEntry.js?param=value'` + ) + ); + + runCLI(`run ${shell}:build:production`); + + // Check the artifact in dist for the remote + const manifestJson = readJson(`dist/apps/${shell}/mf-manifest.json`); + const remoteEntry = manifestJson.remotes[0]; + + expect(remoteEntry).toBeDefined(); + expect(remoteEntry.entry).toContain( + 'http://localhost:4201/remoteEntry.js?param=value' + ); + expect(manifestJson.remotes).toMatchInlineSnapshot(` + [ + { + "alias": "${remote1}", + "entry": "http://localhost:4201/remoteEntry.js?param=value", + "federationContainerName": "${remote1}", + "moduleName": "Module", + }, + ] + `); + + // Update the remote entry to include query params at the end without remoteEntry in path + updateFile(`apps/${shell}/webpack.config.prod.ts`, (content) => + content.replace( + `'http://localhost:4201/remoteEntry.js?param=value'`, + `'http://localhost:4201?param=newValue'` + ) + ); + + runCLI(`run ${shell}:build:production`); + + // Check the artifact in dist for the remote + const manifestJsonUpdated = readJson( + `dist/apps/${shell}/mf-manifest.json` + ); + const remoteEntryUpdated = manifestJsonUpdated.remotes[0]; // There should be only one remote + + expect(remoteEntryUpdated).toBeDefined(); + expect(remoteEntryUpdated.entry).toContain( + 'http://localhost:4201/remoteEntry.js?param=newValue' + ); + }); + describe('ssr', () => { it('should generate host and remote apps with ssr', async () => { const shell = uniq('shell'); diff --git a/packages/angular/mf/mf.ts b/packages/angular/mf/mf.ts index eab3f1d8da..c2558752bc 100644 --- a/packages/angular/mf/mf.ts +++ b/packages/angular/mf/mf.ts @@ -1,3 +1,5 @@ +import { extname } from 'node:path'; + export type ResolveRemoteUrlFunction = ( remoteName: string ) => string | Promise; @@ -131,13 +133,19 @@ async function loadRemoteContainer(remoteName: string) { ? remoteUrlDefinitions[remoteName] : await resolveRemoteUrl(remoteName); - let containerUrl = remoteUrl; - if (!remoteUrl.endsWith('.mjs') && !remoteUrl.endsWith('.js')) { - containerUrl = `${remoteUrl}${ - remoteUrl.endsWith('/') ? '' : '/' - }remoteEntry.mjs`; + const url = new URL(remoteUrl); + const ext = extname(url.pathname); + + const needsRemoteEntry = !['.js', '.mjs', '.json'].includes(ext); + + if (needsRemoteEntry) { + url.pathname = url.pathname.endsWith('/') + ? `${url.pathname}remoteEntry.mjs` + : `${url.pathname}/remoteEntry.mjs`; } + const containerUrl = url.href; + const container = await loadModule(containerUrl); await container.init(__webpack_share_scopes__.default); diff --git a/packages/module-federation/src/utils/remotes.ts b/packages/module-federation/src/utils/remotes.ts index fd77f92e55..38a38f87a3 100644 --- a/packages/module-federation/src/utils/remotes.ts +++ b/packages/module-federation/src/utils/remotes.ts @@ -45,33 +45,27 @@ function handleArrayRemote( remoteEntryExt: 'js' | 'mjs', isRemoteGlobal: boolean ): string { - let [nxRemoteProjectName, remoteLocation] = remote; + const [nxRemoteProjectName, remoteLocation] = remote; const mfRemoteName = normalizeRemoteName(nxRemoteProjectName); - const remoteLocationExt = extname(remoteLocation); - // If remote location already has .js or .mjs extension - if (['.js', '.mjs', '.json'].includes(remoteLocationExt)) { - if (isRemoteGlobal && !remoteLocation.startsWith(`${mfRemoteName}@`)) { - return `${mfRemoteName}@${remoteLocation}`; - } + // Remote string starts like "promise new Promise(...)" – return as-is + if (remoteLocation.startsWith('promise new Promise')) { return remoteLocation; } - const baseRemote = remoteLocation.endsWith('/') - ? remoteLocation.slice(0, -1) - : remoteLocation; + const resolvedUrl = new URL(remoteLocation); + const ext = extname(resolvedUrl.pathname); + const needsRemoteEntry = !['.js', '.mjs', '.json'].includes(ext); - const globalPrefix = isRemoteGlobal - ? `${normalizeRemoteName(nxRemoteProjectName)}@` - : ''; - - // if the remote is defined with anything other than http then we assume it's a promise based remote - // In that case we should use what the user provides as the remote location - if (!remoteLocation.startsWith('promise new Promise')) { - return `${globalPrefix}${baseRemote}/remoteEntry.${remoteEntryExt}`; - } else { - return remoteLocation; + if (needsRemoteEntry) { + resolvedUrl.pathname = resolvedUrl.pathname.endsWith('/') + ? `${resolvedUrl.pathname}remoteEntry.${remoteEntryExt}` + : `${resolvedUrl.pathname}/remoteEntry.${remoteEntryExt}`; } + + const finalRemoteUrl = resolvedUrl.href; + + return isRemoteGlobal ? `${mfRemoteName}@${finalRemoteUrl}` : finalRemoteUrl; } // Helper function to deal with remotes that are strings @@ -106,16 +100,20 @@ export function mapRemotesForSSR( if (Array.isArray(remote)) { let [nxRemoteProjectName, remoteLocation] = remote; const mfRemoteName = normalizeRemoteName(nxRemoteProjectName); - const remoteLocationExt = extname(remoteLocation); - mappedRemotes[mfRemoteName] = `${mfRemoteName}@${ - ['.js', '.mjs', '.json'].includes(remoteLocationExt) - ? remoteLocation - : `${ - remoteLocation.endsWith('/') - ? remoteLocation.slice(0, -1) - : remoteLocation - }/remoteEntry.${remoteEntryExt}` - }`; + + const resolvedUrl = new URL(remoteLocation); + const remoteLocationExt = extname(resolvedUrl.pathname); + const needsRemoteEntry = !['.js', '.mjs', '.json'].includes( + remoteLocationExt + ); + + if (needsRemoteEntry) { + resolvedUrl.pathname = resolvedUrl.pathname.endsWith('/') + ? `${resolvedUrl.pathname}remoteEntry.${remoteEntryExt}` + : `${resolvedUrl.pathname}/remoteEntry.${remoteEntryExt}`; + } + const finalRemoteUrl = resolvedUrl.href; + mappedRemotes[mfRemoteName] = `${mfRemoteName}@${finalRemoteUrl}`; } else if (typeof remote === 'string') { const mfRemoteName = normalizeRemoteName(remote); mappedRemotes[mfRemoteName] = `${mfRemoteName}@${determineRemoteUrl( diff --git a/packages/module-federation/src/with-module-federation/rspack/with-module-federation.ts b/packages/module-federation/src/with-module-federation/rspack/with-module-federation.ts index df5d8b100a..7a01d441e8 100644 --- a/packages/module-federation/src/with-module-federation/rspack/with-module-federation.ts +++ b/packages/module-federation/src/with-module-federation/rspack/with-module-federation.ts @@ -6,7 +6,6 @@ import { NxModuleFederationConfigOverride, } from '../../utils'; import { getModuleFederationConfig } from './utils'; -import { type ExecutorContext } from '@nx/devkit'; const isVarOrWindow = (libType?: string) => libType === 'var' || libType === 'window'; diff --git a/packages/react/mf/dynamic-federation.ts b/packages/react/mf/dynamic-federation.ts index bc1cfb19a6..3b442e93eb 100644 --- a/packages/react/mf/dynamic-federation.ts +++ b/packages/react/mf/dynamic-federation.ts @@ -1,3 +1,5 @@ +import { extname } from 'node:path'; + export type ResolveRemoteUrlFunction = ( remoteName: string ) => string | Promise; @@ -158,13 +160,19 @@ async function loadRemoteContainer(remoteName: string) { ? remoteUrlDefinitions[remoteName] : await resolveRemoteUrl(remoteName); - let containerUrl = remoteUrl; - if (!remoteUrl.endsWith('.mjs') && !remoteUrl.endsWith('.js')) { - containerUrl = `${remoteUrl}${ - remoteUrl.endsWith('/') ? '' : '/' - }remoteEntry.js`; + const url = new URL(remoteUrl); + const ext = extname(url.pathname); + + const needsRemoteEntry = !['.js', '.mjs', '.json'].includes(ext); + + if (needsRemoteEntry) { + url.pathname = url.pathname.endsWith('/') + ? `${url.pathname}remoteEntry.mjs` + : `${url.pathname}/remoteEntry.mjs`; } + const containerUrl = url.href; + const container = await fetchRemoteModule(containerUrl, remoteName); await container.init(__webpack_share_scopes__.default);