From afe4f2fe13490cf2cc6a14bc4b5364ea75659283 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Thu, 16 May 2024 18:27:35 +0200 Subject: [PATCH 1/2] Fix the runtime for rsc layer (#65850) ### What Fix a bug introduced in #65694 , use app-page runtime for app router layers ### Why This is basically reverted the route context picking up logic we had before. During the test we found the error thrown > Module not found: shared-runtime module router-context cannot be used in rsc layer Which is caused by a `next/router` imports in rsc page. Decided to revert to what it was before as the most safe way to load share module contexts. It's caused by `next-contentlayer` usage that they're using `next/router` in server component MDX, but we cannot lint error that from node_modules. (We actually can, but disabled that due to various mis-usage of server/client hooks we had before) --- packages/next/src/build/webpack-config.ts | 17 +++++++++-------- test/e2e/app-dir/navigation/navigation.test.ts | 7 ------- .../app-dir/navigation/pages/api/navigation.js | 5 ----- .../rsc-basic/app/shared-context/server/page.js | 5 +++++ .../app-dir/rsc-basic/pages/api/navigation.js | 6 ++++++ test/e2e/app-dir/rsc-basic/rsc-basic.test.ts | 14 ++++++++++++++ test/turbopack-build-tests-manifest.json | 4 +++- 7 files changed, 37 insertions(+), 21 deletions(-) delete mode 100644 test/e2e/app-dir/navigation/pages/api/navigation.js create mode 100644 test/e2e/app-dir/rsc-basic/app/shared-context/server/page.js create mode 100644 test/e2e/app-dir/rsc-basic/pages/api/navigation.js diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 00c2a202eeedaa..75269fa337eb1c 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1710,14 +1710,15 @@ export default async function getBaseWebpackConfig( const layer = resource.contextInfo.issuerLayer let runtime - if (layer === WEBPACK_LAYERS.serverSideRendering) { - runtime = 'app-page' - } else if (!layer || layer === WEBPACK_LAYERS.api) { - runtime = 'pages' - } else { - throw new Error( - `shared-runtime module ${moduleName} cannot be used in ${layer} layer` - ) + switch (layer) { + case WEBPACK_LAYERS.serverSideRendering: + case WEBPACK_LAYERS.reactServerComponents: + case WEBPACK_LAYERS.appPagesBrowser: + case WEBPACK_LAYERS.actionBrowser: + runtime = 'app-page' + break + default: + runtime = 'pages' } resource.request = `next/dist/server/future/route-modules/${runtime}/vendored/contexts/${moduleName}` } diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts index 316c4740a3b55c..c45de0c4efbf5c 100644 --- a/test/e2e/app-dir/navigation/navigation.test.ts +++ b/test/e2e/app-dir/navigation/navigation.test.ts @@ -909,11 +909,4 @@ describe('app dir - navigation', () => { }) }) }) - - describe('pages api', () => { - it('should not error if just import the navigation api in pages/api', async () => { - const res = await next.fetch('/api/navigation') - expect(res.status).toBe(200) - }) - }) }) diff --git a/test/e2e/app-dir/navigation/pages/api/navigation.js b/test/e2e/app-dir/navigation/pages/api/navigation.js deleted file mode 100644 index ddabe2dd61520e..00000000000000 --- a/test/e2e/app-dir/navigation/pages/api/navigation.js +++ /dev/null @@ -1,5 +0,0 @@ -import { useParams } from 'next/navigation' - -export default function handle(_, res) { - res.send(`${typeof useParams}`) -} diff --git a/test/e2e/app-dir/rsc-basic/app/shared-context/server/page.js b/test/e2e/app-dir/rsc-basic/app/shared-context/server/page.js new file mode 100644 index 00000000000000..8038bc609dfda5 --- /dev/null +++ b/test/e2e/app-dir/rsc-basic/app/shared-context/server/page.js @@ -0,0 +1,5 @@ +require('next/router') + +export default function Page() { + return

just work

+} diff --git a/test/e2e/app-dir/rsc-basic/pages/api/navigation.js b/test/e2e/app-dir/rsc-basic/pages/api/navigation.js new file mode 100644 index 00000000000000..ad34252a337489 --- /dev/null +++ b/test/e2e/app-dir/rsc-basic/pages/api/navigation.js @@ -0,0 +1,6 @@ +// Use `require` to skip the api check +require('next/navigation') + +export default function handle(_, res) { + res.send('just work') +} diff --git a/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts b/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts index a188fc40ec268f..2fd7c3fe78e15a 100644 --- a/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts +++ b/test/e2e/app-dir/rsc-basic/rsc-basic.test.ts @@ -59,6 +59,20 @@ describe('app dir - rsc basics', () => { }) } + describe('next internal shared context', () => { + it('should not error if just load next/navigation module in pages/api', async () => { + const res = await next.fetch('/api/navigation') + expect(res.status).toBe(200) + expect(await res.text()).toBe('just work') + }) + + it('should not error if just load next/router module in app page', async () => { + const res = await next.fetch('/shared-context/server') + expect(res.status).toBe(200) + expect(await res.text()).toContain('just work') + }) + }) + it('should correctly render page returning null', async () => { const homeHTML = await next.render('/return-null/page') const $ = cheerio.load(homeHTML) diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index 2a632273ccf607..93074c1ef51f40 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -2889,7 +2889,9 @@ "app dir - rsc basics should suspense next/image in server components", "app dir - rsc basics should suspense next/legacy/image in server components", "app dir - rsc basics should track client components in dynamic imports", - "app dir - rsc basics should use canary react for app" + "app dir - rsc basics should use canary react for app", + "app dir - rsc basics next internal shared context should not error if just load next/navigation module in pages/api", + "app dir - rsc basics next internal shared context should not error if just load next/router module in app page" ], "pending": [ "app dir - rsc basics should support partial hydration with inlined server data in browser", From aed5242192b65018f5bc5a99b5c1b163305a76a3 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 16 May 2024 09:59:41 -0700 Subject: [PATCH 2/2] Run CNA tests with Next.js from branch (#65852) This is probably not complete but something in that direction. --------- Co-authored-by: Jiachi Liu --- .../create-next-app/examples.test.ts | 47 +++++++++++++++---- .../integration/create-next-app/index.test.ts | 13 +++++ .../package-manager/bun.test.ts | 25 ++++++++-- .../package-manager/npm.test.ts | 25 ++++++++-- .../package-manager/pnpm.test.ts | 25 ++++++++-- .../package-manager/yarn.test.ts | 18 +++++-- test/integration/create-next-app/utils.ts | 10 ++-- 7 files changed, 133 insertions(+), 30 deletions(-) diff --git a/test/integration/create-next-app/examples.test.ts b/test/integration/create-next-app/examples.test.ts index 64bd81ecb141af..b37d214d97c071 100644 --- a/test/integration/create-next-app/examples.test.ts +++ b/test/integration/create-next-app/examples.test.ts @@ -1,3 +1,5 @@ +import { trace } from 'next/dist/trace' +import { createNextInstall } from '../../lib/create-next-install' import { EXAMPLE_PATH, EXAMPLE_REPO, @@ -10,12 +12,23 @@ import { } from './utils' describe('create-next-app --example', () => { + let nextInstall: Awaited> + beforeAll(async () => { + nextInstall = await createNextInstall({ + parentSpan: trace('test'), + keepRepoDir: Boolean(process.env.NEXT_TEST_SKIP_CLEANUP), + }) + }) it('should create on valid Next.js example name', async () => { await useTempDir(async (cwd) => { const projectName = 'valid-example' - const res = await run([projectName, '--example', 'basic-css'], { - cwd, - }) + const res = await run( + [projectName, '--example', 'basic-css'], + nextInstall.installDir, + { + cwd, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ cwd, @@ -34,9 +47,13 @@ describe('create-next-app --example', () => { it('should create with GitHub URL', async () => { await useTempDir(async (cwd) => { const projectName = 'github-url' - const res = await run([projectName, '--example', FULL_EXAMPLE_PATH], { - cwd, - }) + const res = await run( + [projectName, '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, + { + cwd, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ @@ -64,6 +81,7 @@ describe('create-next-app --example', () => { // GH#39665 'https://github.com/vercel/nextjs-portfolio-starter/', ], + nextInstall.installDir, { cwd, } @@ -97,6 +115,7 @@ describe('create-next-app --example', () => { '--example-path', EXAMPLE_PATH, ], + nextInstall.installDir, { cwd, } @@ -131,6 +150,7 @@ describe('create-next-app --example', () => { '--example-path', EXAMPLE_PATH, ], + nextInstall.installDir, { cwd, } @@ -168,6 +188,7 @@ describe('create-next-app --example', () => { '__internal-testing-retry', '--import-alias=@/*', ], + nextInstall.installDir, { cwd, input: '\n', // 'Yes' to retry @@ -199,6 +220,7 @@ describe('create-next-app --example', () => { 'default', '--import-alias=@/*', ], + nextInstall.installDir, { cwd, } @@ -217,10 +239,14 @@ describe('create-next-app --example', () => { it('should not create if --example flag value is invalid', async () => { await useTempDir(async (cwd) => { const projectName = 'invalid-example' - const res = await run([projectName, '--example', 'not a real example'], { - cwd, - reject: false, - }) + const res = await run( + [projectName, '--example', 'not a real example'], + nextInstall.installDir, + { + cwd, + reject: false, + } + ) expect(res.exitCode).toBe(1) projectFilesShouldNotExist({ @@ -244,6 +270,7 @@ describe('create-next-app --example', () => { '--no-tailwind', '--example', ], + nextInstall.installDir, { cwd, reject: false, diff --git a/test/integration/create-next-app/index.test.ts b/test/integration/create-next-app/index.test.ts index ca008dbf66317b..09ae4757e170f2 100644 --- a/test/integration/create-next-app/index.test.ts +++ b/test/integration/create-next-app/index.test.ts @@ -6,6 +6,16 @@ import { projectFilesShouldExist, projectFilesShouldNotExist, } from './utils' +import { createNextInstall } from '../../lib/create-next-install' +import { trace } from 'next/dist/trace' + +let nextInstall: Awaited> +beforeAll(async () => { + nextInstall = await createNextInstall({ + parentSpan: trace('test'), + keepRepoDir: Boolean(process.env.NEXT_TEST_SKIP_CLEANUP), + }) +}) describe('create-next-app', () => { it('should not create if the target directory is not empty', async () => { @@ -25,6 +35,7 @@ describe('create-next-app', () => { '--no-src-dir', '--no-import-alias', ], + nextInstall.installDir, { cwd, reject: false, @@ -64,6 +75,7 @@ describe('create-next-app', () => { '--no-src-dir', '--no-import-alias', ], + nextInstall.installDir, { cwd, reject: false, @@ -93,6 +105,7 @@ describe('create-next-app', () => { '--no-import-alias', '--skip-install', ], + nextInstall.installDir, { cwd, } diff --git a/test/integration/create-next-app/package-manager/bun.test.ts b/test/integration/create-next-app/package-manager/bun.test.ts index fa1973a66030f8..0000597ab5e50f 100644 --- a/test/integration/create-next-app/package-manager/bun.test.ts +++ b/test/integration/create-next-app/package-manager/bun.test.ts @@ -1,3 +1,5 @@ +import { trace } from 'next/dist/trace' +import { createNextInstall } from '../../../lib/create-next-install' import { command, DEFAULT_FILES, @@ -16,6 +18,14 @@ beforeEach(async () => { .catch(() => command('npm', ['i', '-g', 'bun'])) }) +let nextInstall: Awaited> +beforeAll(async () => { + nextInstall = await createNextInstall({ + parentSpan: trace('test'), + keepRepoDir: Boolean(process.env.NEXT_TEST_SKIP_CLEANUP), + }) +}) + describe('create-next-app with package manager bun', () => { it('should use bun for --use-bun flag', async () => { await useTempDir(async (cwd) => { @@ -31,6 +41,7 @@ describe('create-next-app with package manager bun', () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, } @@ -59,6 +70,7 @@ it('should use bun when user-agent is bun', async () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, env: { npm_config_user_agent: 'bun' }, @@ -79,6 +91,7 @@ it('should use bun for --use-bun flag with example', async () => { const projectName = 'use-bun-with-example' const res = await run( [projectName, '--use-bun', '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, { cwd } ) @@ -94,10 +107,14 @@ it('should use bun for --use-bun flag with example', async () => { it('should use bun when user-agent is bun with example', async () => { await useTempDir(async (cwd) => { const projectName = 'user-agent-bun-with-example' - const res = await run([projectName, '--example', FULL_EXAMPLE_PATH], { - cwd, - env: { npm_config_user_agent: 'bun' }, - }) + const res = await run( + [projectName, '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, + { + cwd, + env: { npm_config_user_agent: 'bun' }, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ diff --git a/test/integration/create-next-app/package-manager/npm.test.ts b/test/integration/create-next-app/package-manager/npm.test.ts index cafbf60cf60e7e..7cdffabb894960 100644 --- a/test/integration/create-next-app/package-manager/npm.test.ts +++ b/test/integration/create-next-app/package-manager/npm.test.ts @@ -1,3 +1,4 @@ +import { trace } from 'next/dist/trace' import { DEFAULT_FILES, FULL_EXAMPLE_PATH, @@ -5,10 +6,19 @@ import { run, useTempDir, } from '../utils' +import { createNextInstall } from '../../../lib/create-next-install' const lockFile = 'package-lock.json' const files = [...DEFAULT_FILES, lockFile] +let nextInstall: Awaited> +beforeAll(async () => { + nextInstall = await createNextInstall({ + parentSpan: trace('test'), + keepRepoDir: Boolean(process.env.NEXT_TEST_SKIP_CLEANUP), + }) +}) + describe('create-next-app with package manager npm', () => { it('should use npm for --use-npm flag', async () => { await useTempDir(async (cwd) => { @@ -24,6 +34,7 @@ describe('create-next-app with package manager npm', () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, } @@ -52,6 +63,7 @@ it('should use npm when user-agent is npm', async () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, env: { npm_config_user_agent: 'npm' }, @@ -72,6 +84,7 @@ it('should use npm for --use-npm flag with example', async () => { const projectName = 'use-npm-with-example' const res = await run( [projectName, '--use-npm', '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, { cwd } ) @@ -87,10 +100,14 @@ it('should use npm for --use-npm flag with example', async () => { it('should use npm when user-agent is npm with example', async () => { await useTempDir(async (cwd) => { const projectName = 'user-agent-npm-with-example' - const res = await run([projectName, '--example', FULL_EXAMPLE_PATH], { - cwd, - env: { npm_config_user_agent: 'npm' }, - }) + const res = await run( + [projectName, '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, + { + cwd, + env: { npm_config_user_agent: 'npm' }, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ diff --git a/test/integration/create-next-app/package-manager/pnpm.test.ts b/test/integration/create-next-app/package-manager/pnpm.test.ts index fa44eacc907a07..3b788f3b77ba90 100644 --- a/test/integration/create-next-app/package-manager/pnpm.test.ts +++ b/test/integration/create-next-app/package-manager/pnpm.test.ts @@ -1,3 +1,5 @@ +import { trace } from 'next/dist/trace' +import { createNextInstall } from '../../../lib/create-next-install' import { command, DEFAULT_FILES, @@ -10,6 +12,14 @@ import { const lockFile = 'pnpm-lock.yaml' const files = [...DEFAULT_FILES, lockFile] +let nextInstall: Awaited> +beforeAll(async () => { + nextInstall = await createNextInstall({ + parentSpan: trace('test'), + keepRepoDir: Boolean(process.env.NEXT_TEST_SKIP_CLEANUP), + }) +}) + beforeEach(async () => { await command('pnpm', ['--version']) // install pnpm if not available @@ -32,6 +42,7 @@ describe('create-next-app with package manager pnpm', () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, } @@ -60,6 +71,7 @@ it('should use pnpm when user-agent is pnpm', async () => { '--no-tailwind', '--no-import-alias', ], + nextInstall.installDir, { cwd, env: { npm_config_user_agent: 'pnpm' }, @@ -80,6 +92,7 @@ it('should use pnpm for --use-pnpm flag with example', async () => { const projectName = 'use-pnpm-with-example' const res = await run( [projectName, '--use-pnpm', '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, { cwd } ) @@ -95,10 +108,14 @@ it('should use pnpm for --use-pnpm flag with example', async () => { it('should use pnpm when user-agent is pnpm with example', async () => { await useTempDir(async (cwd) => { const projectName = 'user-agent-pnpm-with-example' - const res = await run([projectName, '--example', FULL_EXAMPLE_PATH], { - cwd, - env: { npm_config_user_agent: 'pnpm' }, - }) + const res = await run( + [projectName, '--example', FULL_EXAMPLE_PATH], + nextInstall.installDir, + { + cwd, + env: { npm_config_user_agent: 'pnpm' }, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ diff --git a/test/integration/create-next-app/package-manager/yarn.test.ts b/test/integration/create-next-app/package-manager/yarn.test.ts index 09dde41a67bc34..a821bd35829a57 100644 --- a/test/integration/create-next-app/package-manager/yarn.test.ts +++ b/test/integration/create-next-app/package-manager/yarn.test.ts @@ -10,6 +10,9 @@ import { const lockFile = 'yarn.lock' const files = [...DEFAULT_FILES, lockFile] +// Don't install local next build here as yarn will error with: +// Usage Error: This project is configured to use pnpm + beforeEach(async () => { await command('yarn', ['--version']) // install yarn if not available @@ -32,6 +35,7 @@ describe('create-next-app with package manager yarn', () => { '--no-tailwind', '--no-import-alias', ], + 'canary', { cwd, } @@ -60,6 +64,7 @@ it('should use yarn when user-agent is yarn', async () => { '--no-tailwind', '--no-import-alias', ], + 'canary', { cwd, env: { npm_config_user_agent: 'yarn' }, @@ -80,6 +85,7 @@ it('should use yarn for --use-yarn flag with example', async () => { const projectName = 'use-yarn-with-example' const res = await run( [projectName, '--use-yarn', '--example', FULL_EXAMPLE_PATH], + 'canary', { cwd } ) @@ -95,10 +101,14 @@ it('should use yarn for --use-yarn flag with example', async () => { it('should use yarn when user-agent is yarn with example', async () => { await useTempDir(async (cwd) => { const projectName = 'user-agent-yarn-with-example' - const res = await run([projectName, '--example', FULL_EXAMPLE_PATH], { - cwd, - env: { npm_config_user_agent: 'yarn' }, - }) + const res = await run( + [projectName, '--example', FULL_EXAMPLE_PATH], + 'canary', + { + cwd, + env: { npm_config_user_agent: 'yarn' }, + } + ) expect(res.exitCode).toBe(0) projectFilesShouldExist({ diff --git a/test/integration/create-next-app/utils.ts b/test/integration/create-next-app/utils.ts index 648ab7dc1b7476..5b1fe07cf67f29 100644 --- a/test/integration/create-next-app/utils.ts +++ b/test/integration/create-next-app/utils.ts @@ -14,25 +14,27 @@ export const DEFAULT_FILES = [ 'node_modules/next', ] -export const run = ( +export const run = async ( args: string[], + nextJSVersion: string, options: | execa.Options | { reject?: boolean env?: Record } -) => - execa('node', [CNA_PATH].concat(args), { +) => { + return execa('node', [CNA_PATH].concat(args), { // tests with options.reject false are expected to exit(1) so don't inherit stdio: options.reject === false ? 'pipe' : 'inherit', ...options, env: { ...process.env, ...options.env, - NEXT_PRIVATE_TEST_VERSION: 'canary', + NEXT_PRIVATE_TEST_VERSION: nextJSVersion, }, }) +} export const command = (cmd: string, args: string[]) => execa(cmd, args, {