From 8552a4a4054b097d6063f87ec82087ea54609ae1 Mon Sep 17 00:00:00 2001 From: Mikhail Bashkirov Date: Fri, 3 May 2024 21:16:40 +0200 Subject: [PATCH] fix(dev-server-rollup): dedupe imports from outside root --- .changeset/witty-ways-occur.md | 5 +++ .../dev-server-rollup/src/rollupAdapter.ts | 17 ++++++-- .../node_modules/storybook/index.js | 1 + .../node_modules/storybook/package.json | 3 ++ .../src/packages/subpackage/app.js | 1 + .../.prebundled_modules/react.mjs | 1 + .../dev-server-rollup/test/node/unit.test.ts | 40 +++++++++++++++++++ 7 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 .changeset/witty-ways-occur.md create mode 100644 packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/index.js create mode 100644 packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/package.json create mode 100644 packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/app.js create mode 100644 packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/node_modules/.prebundled_modules/react.mjs diff --git a/.changeset/witty-ways-occur.md b/.changeset/witty-ways-occur.md new file mode 100644 index 000000000..52aa0402a --- /dev/null +++ b/.changeset/witty-ways-occur.md @@ -0,0 +1,5 @@ +--- +'@web/dev-server-rollup': patch +--- + +dedupe imports from outside root diff --git a/packages/dev-server-rollup/src/rollupAdapter.ts b/packages/dev-server-rollup/src/rollupAdapter.ts index 96c45a40a..94d2b9581 100644 --- a/packages/dev-server-rollup/src/rollupAdapter.ts +++ b/packages/dev-server-rollup/src/rollupAdapter.ts @@ -289,9 +289,9 @@ export function rollupAdapter( // append a path separator to rootDir so we are actually testing containment // of the normalized path within the rootDir folder - const checkRootDir = rootDir.endsWith(path.sep) ? rootDir : rootDir + path.sep; + const normalizedRootDir = rootDir.endsWith(path.sep) ? rootDir : rootDir + path.sep; - if (!normalizedPath.startsWith(checkRootDir)) { + if (!normalizedPath.startsWith(normalizedRootDir)) { const relativePath = path.relative(rootDir, normalizedPath); const dirUp = `..${path.sep}`; const lastDirUpIndex = relativePath.lastIndexOf(dirUp) + 3; @@ -317,8 +317,17 @@ export function rollupAdapter( const importPath = toBrowserPath(relativePath.substring(lastDirUpIndex)); resolvedImportPath = `/__wds-outside-root__/${dirUpStrings.length - 1}/${importPath}`; } else { - const resolveRelativeTo = path.dirname(filePath); - const relativeImportFilePath = path.relative(resolveRelativeTo, resolvedImportPath); + let relativeImportFilePath = ''; + + if (context.url.match(OUTSIDE_ROOT_REGEXP)) { + const pathInsideRootDir = `/${normalizedPath.replace(normalizedRootDir, '')}`; + const resolveRelativeTo = path.dirname(context.url); + relativeImportFilePath = path.relative(resolveRelativeTo, pathInsideRootDir); + } else { + const resolveRelativeTo = path.dirname(filePath); + relativeImportFilePath = path.relative(resolveRelativeTo, resolvedImportPath); + } + resolvedImportPath = `./${toBrowserPath(relativeImportFilePath)}`; } diff --git a/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/index.js b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/index.js new file mode 100644 index 000000000..83bcf4d10 --- /dev/null +++ b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/index.js @@ -0,0 +1 @@ +import 'react'; diff --git a/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/package.json b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/package.json new file mode 100644 index 000000000..82e984d69 --- /dev/null +++ b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/node_modules/storybook/package.json @@ -0,0 +1,3 @@ +{ + "name": "storybook" +} \ No newline at end of file diff --git a/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/app.js b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/app.js new file mode 100644 index 000000000..83bcf4d10 --- /dev/null +++ b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/app.js @@ -0,0 +1 @@ +import 'react'; diff --git a/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/node_modules/.prebundled_modules/react.mjs b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/node_modules/.prebundled_modules/react.mjs new file mode 100644 index 000000000..fcc5f7b09 --- /dev/null +++ b/packages/dev-server-rollup/test/node/fixtures/monorepo-import-inside-from-outside/src/packages/subpackage/node_modules/.prebundled_modules/react.mjs @@ -0,0 +1 @@ +export default 'react bundle'; \ No newline at end of file diff --git a/packages/dev-server-rollup/test/node/unit.test.ts b/packages/dev-server-rollup/test/node/unit.test.ts index 0271cdcd2..073b2ab67 100644 --- a/packages/dev-server-rollup/test/node/unit.test.ts +++ b/packages/dev-server-rollup/test/node/unit.test.ts @@ -104,6 +104,46 @@ describe('@web/dev-server-rollup', () => { }); }); + it('files inside root directory imported by files inside or outside are resolved to be deduped in the browser', async () => { + const rootDir = path.resolve( + __dirname, + 'fixtures', + 'monorepo-import-inside-from-outside', + 'src', + 'packages', + 'subpackage', + ); + const { server, host } = await createTestServer({ + rootDir, + plugins: [ + fromRollup(() => { + return { + name: 'prebundle-modules', + async resolveId(source) { + if (source === 'react') { + return path.join(rootDir, 'node_modules', '.prebundled_modules', 'react.mjs'); + } + }, + }; + })(), + ], + }); + + try { + const responseTextInside = await fetchText(`${host}/app.js`); + expectIncludes(responseTextInside, "import './node_modules/.prebundled_modules/react.mjs'"); + const responseTextOutside = await fetchText( + `${host}/__wds-outside-root__/3/node_modules/storybook/index.js`, + ); + expectIncludes( + responseTextOutside, + "import './../../../../node_modules/.prebundled_modules/react.mjs'", + ); + } finally { + server.stop(); + } + }); + describe('load', () => { it('can serve files', async () => { const plugin: RollupPlugin = {