From e650047d4b79d130292e75e89f43d127a2d8b4e2 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 25 Nov 2022 02:19:21 +0100 Subject: [PATCH] Fix package resolution issue in app dir (#43349) The server and client compilers have different external dependency resolution logic. The server prefers CJS while the client prefers ESM by default. This causes an issue when that dependency is a client component entry. We get the module reference during RSC render, which is the CJS file path. And that module reference can't be located in the flight manifest as it was generated via the client compiler (has the ESM path). This PR changes the entry creation logic to use resolved requests, instead of the raw requests. So both SSR and client will have the same resolved import paths to ensure the module reference consistency. ## Bug - [ ] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) --- .../plugins/flight-client-entry-plugin.ts | 23 ++++++------------- test/e2e/app-dir/app-external.test.ts | 5 ++++ .../app-external/app/client-dep/page.js | 9 ++++++++ .../node_modules_bak/client-module/index.js | 3 +++ .../node_modules_bak/client-module/index.mjs | 3 +++ .../client-module/package.json | 5 ++++ 6 files changed, 32 insertions(+), 16 deletions(-) create mode 100644 test/e2e/app-dir/app-external/app/client-dep/page.js create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/client-module/index.js create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/client-module/index.mjs create mode 100644 test/e2e/app-dir/app-external/node_modules_bak/client-module/package.json diff --git a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts index 77545f5c2b8..b8c483ceba3 100644 --- a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts @@ -12,7 +12,7 @@ import { entries, EntryTypes, } from '../../../server/dev/on-demand-entry-handler' -import { APP_DIR_ALIAS, WEBPACK_LAYERS } from '../../../lib/constants' +import { WEBPACK_LAYERS } from '../../../lib/constants' import { APP_CLIENT_INTERNALS, COMPILER_NAMES, @@ -379,23 +379,14 @@ export class FlightClientEntryPlugin { compilation.moduleGraph.getResolvedModule(dependencyToFilter) if (!mod) return - // Keep client imports as simple - // native or installed js module: -> raw request, e.g. next/head - // client js or css: -> user request const rawRequest = mod.rawRequest - // Request could be undefined or '' - if (!rawRequest) return - const isCSS = regexCSS.test(rawRequest) - const isLocal = - !isCSS && - !rawRequest.startsWith('.') && - !rawRequest.startsWith('/') && - !rawRequest.startsWith(APP_DIR_ALIAS) - - const modRequest: string | undefined = isLocal - ? rawRequest - : mod.resourceResolveData?.path + mod.resourceResolveData?.query + + // We have to always use the resolved request here to make sure the + // server and client are using the same module path (required by RSC), as + // the server compiler and client compiler have different resolve configs. + const modRequest: string | undefined = + mod.resourceResolveData?.path + mod.resourceResolveData?.query // Ensure module is not walked again if it's already been visited if (!visitedBySegment[entryRequest]) { diff --git a/test/e2e/app-dir/app-external.test.ts b/test/e2e/app-dir/app-external.test.ts index f80c78aacc8..a30ffc420ba 100644 --- a/test/e2e/app-dir/app-external.test.ts +++ b/test/e2e/app-dir/app-external.test.ts @@ -147,6 +147,11 @@ describe('app dir - external dependency', () => { ).toBe('rgb(255, 0, 0)') }) + it('should use the same export type for packages in both ssr and client', async () => { + const browser = await webdriver(next.url, '/client-dep') + expect(await browser.eval(`window.document.body.innerText`)).toBe('hello') + }) + it('should handle external css modules in pages', async () => { const browser = await webdriver(next.url, '/test-pages') diff --git a/test/e2e/app-dir/app-external/app/client-dep/page.js b/test/e2e/app-dir/app-external/app/client-dep/page.js new file mode 100644 index 00000000000..ad1857f515b --- /dev/null +++ b/test/e2e/app-dir/app-external/app/client-dep/page.js @@ -0,0 +1,9 @@ +import Foo from 'client-module' + +export default function Index() { + return ( +
+ +
+ ) +} diff --git a/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.js b/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.js new file mode 100644 index 00000000000..d992d053fe4 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.js @@ -0,0 +1,3 @@ +'use client' + +module.exports = () => 'hello' diff --git a/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.mjs b/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.mjs new file mode 100644 index 00000000000..0572a662841 --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/client-module/index.mjs @@ -0,0 +1,3 @@ +'use client' + +export default () => 'hello' diff --git a/test/e2e/app-dir/app-external/node_modules_bak/client-module/package.json b/test/e2e/app-dir/app-external/node_modules_bak/client-module/package.json new file mode 100644 index 00000000000..d1b194ceeae --- /dev/null +++ b/test/e2e/app-dir/app-external/node_modules_bak/client-module/package.json @@ -0,0 +1,5 @@ +{ + "name": "client-module", + "main": "index.js", + "module": "index.mjs" +}