From afa977217528b2e0f5356984c30d78df98258dc4 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 4 Aug 2025 12:08:56 +0200 Subject: [PATCH 1/3] Turbopack: set env in tracing context (#75254) Part of PACK-4164 ~~Set `NODE_ENV` in tracing context~~ For that, see https://github.com/vercel/next.js/pull/75254#issuecomment-2627686346 Set `TURBOPACK` in tracing context This get rids of some constant build overhead (because we only analyze one instance of react-dom for NFT now, instead of all of the edge/server/client onces). Otherwise no measureable perf/memory impact --- crates/next-core/src/next_server/context.rs | 56 ++++++++++++++++++- turbopack/crates/turbopack/src/lib.rs | 30 +++++----- .../module_options/module_options_context.rs | 18 +++--- 3 files changed, 79 insertions(+), 25 deletions(-) diff --git a/crates/next-core/src/next_server/context.rs b/crates/next-core/src/next_server/context.rs index cdbd39e29d9850..b29e3945b334c1 100644 --- a/crates/next-core/src/next_server/context.rs +++ b/crates/next-core/src/next_server/context.rs @@ -8,8 +8,8 @@ use turbo_tasks_fs::FileSystemPath; use turbopack::{ css::chunk::CssChunkType, module_options::{ - CssOptionsContext, EcmascriptOptionsContext, JsxTransformOptions, ModuleOptionsContext, - ModuleRule, TypeofWindow, TypescriptTransformOptions, + CssOptionsContext, EcmascriptOptionsContext, ExternalsTracingOptions, JsxTransformOptions, + ModuleOptionsContext, ModuleRule, TypeofWindow, TypescriptTransformOptions, }, resolve_options_context::ResolveOptionsContext, transition::Transition, @@ -19,6 +19,7 @@ use turbopack_core::{ ChunkingConfig, MangleType, MinifyType, SourceMapsType, module_id_strategies::ModuleIdStrategy, }, + compile_time_defines, compile_time_info::{CompileTimeDefines, CompileTimeInfo, FreeVarReferences}, environment::{ Environment, ExecutionEnvironment, NodeJsEnvironment, NodeJsVersion, RuntimeVersions, @@ -387,6 +388,49 @@ pub async fn get_server_compile_time_info( .await } +#[turbo_tasks::function] +pub async fn get_tracing_compile_time_info() -> Result> { + CompileTimeInfo::builder( + Environment::new(ExecutionEnvironment::NodeJsLambda( + NodeJsEnvironment::default().resolved_cell(), + )) + .to_resolved() + .await?, + ) + /* + We'd really like to set `process.env.NODE_ENV = "production"` here, but with that, + `react/cjs/react.development.js` won't be copied anymore (as expected). + However if you `import` react from native ESM: `import {createContext} from 'react';`, it fails with + ``` + import {createContext} from 'react'; + ^^^^^^^^^^^^^ + SyntaxError: Named export 'createContext' not found. The requested module 'react' is a CommonJS module, which may not support all module.exports as named exports. + CommonJS modules can always be imported via the default export, for example using: + ``` + This is because Node's import-cjs-from-esm feature can correctly find all named exports in + ``` + // `react/index.js` + if (process.env.NODE_ENV === 'production') { + module.exports = require('./cjs/react.production.js'); + } else { + module.exports = require('./cjs/react.development.js'); + } + ``` + if both files exist (which is what's happening so far). + If `react.development.js` doesn't exist, then it bails with that error message. + Also just removing that second branch works fine, but a `require` to a non-existent file fails. + */ + .defines( + compile_time_defines!( + process.env.TURBOPACK = true, + // process.env.NODE_ENV = "production", + ) + .resolved_cell(), + ) + .cell() + .await +} + #[turbo_tasks::function] pub async fn get_server_module_options_context( project_path: FileSystemPath, @@ -544,7 +588,13 @@ pub async fn get_server_module_options_context( tree_shaking_mode: tree_shaking_mode_for_user_code, side_effect_free_packages: next_config.optimize_package_imports().owned().await?, enable_externals_tracing: if next_mode.is_production() { - Some(project_path) + Some( + ExternalsTracingOptions { + tracing_root: project_path, + compile_time_info: get_tracing_compile_time_info().to_resolved().await?, + } + .resolved_cell(), + ) } else { None }, diff --git a/turbopack/crates/turbopack/src/lib.rs b/turbopack/crates/turbopack/src/lib.rs index 15f452d24ef791..eebe8672d570a9 100644 --- a/turbopack/crates/turbopack/src/lib.rs +++ b/turbopack/crates/turbopack/src/lib.rs @@ -34,7 +34,6 @@ use turbopack_core::{ chunk::SourceMapsType, compile_time_info::CompileTimeInfo, context::{AssetContext, ProcessResult}, - environment::{Environment, ExecutionEnvironment, NodeJsEnvironment}, ident::Layer, issue::{IssueExt, IssueSource, StyledString, module::ModuleIssue}, module::Module, @@ -643,15 +642,12 @@ async fn process_default_internal( } #[turbo_tasks::function] -async fn externals_tracing_module_context(ty: ExternalType) -> Result> { - let env = Environment::new(ExecutionEnvironment::NodeJsLambda( - NodeJsEnvironment::default().resolved_cell(), - )) - .to_resolved() - .await?; - +async fn externals_tracing_module_context( + ty: ExternalType, + compile_time_info: Vc, +) -> Result> { let resolve_options = ResolveOptionsContext { - emulate_environment: Some(env), + emulate_environment: Some(compile_time_info.await?.environment), loose_errors: true, custom_conditions: match ty { ExternalType::CommonJs => vec![rcstr!("require")], @@ -663,7 +659,7 @@ async fn externals_tracing_module_context(ty: ExternalType) -> Result { let replacement = if replace_externals { let tracing_mode = if traced == ExternalTraced::Traced - && let Some(tracing_root) = &self + && let Some(options) = &self .module_options_context() .await? .enable_externals_tracing @@ -793,13 +789,17 @@ impl AssetContext for ModuleAssetContext { // request will later be resolved relative to tracing_root // anyway. + let options = options.await?; CachedExternalTracingMode::Traced { externals_context: ResolvedVc::upcast( - externals_tracing_module_context(ty) - .to_resolved() - .await?, + externals_tracing_module_context( + ty, + *options.compile_time_info, + ) + .to_resolved() + .await?, ), - root_origin: tracing_root.join("_")?, + root_origin: options.tracing_root.join("_")?, } } else { CachedExternalTracingMode::Untraced diff --git a/turbopack/crates/turbopack/src/module_options/module_options_context.rs b/turbopack/crates/turbopack/src/module_options/module_options_context.rs index ce3fc2b831f0cb..3f968f2b2a5c04 100644 --- a/turbopack/crates/turbopack/src/module_options/module_options_context.rs +++ b/turbopack/crates/turbopack/src/module_options/module_options_context.rs @@ -6,8 +6,8 @@ use turbo_rcstr::RcStr; use turbo_tasks::{FxIndexMap, NonLocalValue, ResolvedVc, ValueDefault, Vc, trace::TraceRawVcs}; use turbo_tasks_fs::FileSystemPath; use turbopack_core::{ - chunk::SourceMapsType, condition::ContextCondition, environment::Environment, - resolve::options::ImportMapping, + chunk::SourceMapsType, compile_time_info::CompileTimeInfo, condition::ContextCondition, + environment::Environment, resolve::options::ImportMapping, }; use turbopack_ecmascript::{TreeShakingMode, references::esm::UrlRewriteBehavior}; pub use turbopack_mdx::MdxTransformOptions; @@ -121,7 +121,6 @@ impl ValueDefault for TypescriptTransformOptions { } } -// [TODO]: should enabled_react_refresh belong to this options? #[turbo_tasks::value(shared)] #[derive(Default, Clone, Debug)] pub struct JsxTransformOptions { @@ -131,6 +130,14 @@ pub struct JsxTransformOptions { pub runtime: Option, } +#[turbo_tasks::value(shared)] +#[derive(Clone, Debug)] +pub struct ExternalsTracingOptions { + /// The directory from which the bundled files will require the externals at runtime. + pub tracing_root: FileSystemPath, + pub compile_time_info: ResolvedVc, +} + #[turbo_tasks::value(shared)] #[derive(Clone, Default)] #[serde(default)] @@ -152,10 +159,7 @@ pub struct ModuleOptionsContext { /// Generate (non-emitted) output assets for static assets and externals, to facilitate /// generating a list of all non-bundled files that will be required at runtime. - /// - /// The filepath is the directory from which the bundled files will require the externals at - /// runtime. - pub enable_externals_tracing: Option, + pub enable_externals_tracing: Option>, /// If true, it stores the last successful parse result in state and keeps using it when /// parsing fails. This is useful to keep the module graph structure intact when syntax errors From b3229324d488e66ea5d8171ce09c2f69f8b45091 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 4 Aug 2025 13:33:06 +0200 Subject: [PATCH 2/3] [test] Update snapshots (#82327) --- .../app-dir/ssr-in-rsc/ssr-in-rsc.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/development/app-dir/ssr-in-rsc/ssr-in-rsc.test.ts b/test/development/app-dir/ssr-in-rsc/ssr-in-rsc.test.ts index 09575af6939edb..ddf157bc0ae5c8 100644 --- a/test/development/app-dir/ssr-in-rsc/ssr-in-rsc.test.ts +++ b/test/development/app-dir/ssr-in-rsc/ssr-in-rsc.test.ts @@ -794,16 +794,16 @@ describe('react-dom/server in React Server environment', () => { if (isTurbopack) { if (isReact18) { expect(redbox).toMatchInlineSnapshot(` - { - "description": "Cannot read properties of undefined (reading 'ReactCurrentDispatcher')", - "source": "internal-pkg/server.node.js (1:1) @ {module evaluation} + { + "description": "Cannot read properties of undefined (reading 'ReactCurrentDispatcher')", + "source": "internal-pkg/server.node.js (1:1) @ {module evaluation} - > 1 | import * as ReactDOMServerEdge from 'react-dom/server.node' - | ^ - 2 | // Fine to drop once React is on ESM - 3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node' - 4 |", - } + > 1 | import * as ReactDOMServerEdge from 'react-dom/server.node' + | ^ + 2 | // Fine to drop once React is on ESM + 3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node' + 4 |", + } `) } else { expect(redbox).toMatchInlineSnapshot(` From 5bb867a9f19aa18329b60232bd1ae2f41d3742f6 Mon Sep 17 00:00:00 2001 From: Janka Uryga Date: Mon, 4 Aug 2025 14:21:20 +0200 Subject: [PATCH 3/3] fix: next/root-params erroring when rerendering after action (#82326) When implementing the check that disallows using `next/root-params` in server actions, i forgot that when a page is rerendered after an action, the render is still wrapped with an `actionAsyncStorage` with `actionStore.isAction === true`. The fix is to also check that we're in the `'action'` phase, i.e. we haven't switched to rendering yet. Closes #82302 --- packages/next/errors.json | 3 +- .../next/src/server/request/root-params.ts | 19 +++--- .../rerender-after-server-action/page.tsx | 37 +++++++++++ .../app-root-params-getters/simple.test.ts | 64 ++++++++++++++++++- 4 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 test/e2e/app-dir/app-root-params-getters/fixtures/simple/app/[lang]/[locale]/rerender-after-server-action/page.tsx diff --git a/packages/next/errors.json b/packages/next/errors.json index 9e155b1db1bfc3..0be08c6e6ea665 100644 --- a/packages/next/errors.json +++ b/packages/next/errors.json @@ -771,5 +771,6 @@ "770": "createParamsFromClient should not be called in a runtime prerender.", "771": "\\`%s\\` was called during a runtime prerender. Next.js should be preventing %s from being included in server components statically, but did not in this case.", "772": "FetchStrategy.PPRRuntime should never be used when `experimental.clientSegmentCache` is disabled", - "773": "Missing workStore in createPrerenderParamsForClientSegment" + "773": "Missing workStore in createPrerenderParamsForClientSegment", + "774": "Route %s used %s outside of a Server Component. This is not allowed." } diff --git a/packages/next/src/server/request/root-params.ts b/packages/next/src/server/request/root-params.ts index 56f00fac2efadf..9bb919a714dd44 100644 --- a/packages/next/src/server/request/root-params.ts +++ b/packages/next/src/server/request/root-params.ts @@ -203,6 +203,13 @@ export function getRootParam(paramName: string): Promise { throw new InvariantError(`Missing workStore in ${apiName}`) } + const workUnitStore = workUnitAsyncStorage.getStore() + if (!workUnitStore) { + throw new Error( + `Route ${workStore.route} used ${apiName} outside of a Server Component. This is not allowed.` + ) + } + const actionStore = actionAsyncStorage.getStore() if (actionStore) { if (actionStore.isAppRoute) { @@ -211,23 +218,17 @@ export function getRootParam(paramName: string): Promise { `Route ${workStore.route} used ${apiName} inside a Route Handler. Support for this API in Route Handlers is planned for a future version of Next.js.` ) } - if (actionStore.isAction) { + if (actionStore.isAction && workUnitStore.phase === 'action') { // Actions are not fundamentally tied to a route (even if they're always submitted from some page), // so root params would be inconsistent if an action is called from multiple roots. + // Make sure we check if the phase is "action" - we should not error in the rerender + // after an action revalidates or updates cookies (which will still have `actionStore.isAction === true`) throw new Error( `${apiName} was used inside a Server Action. This is not supported. Functions from 'next/root-params' can only be called in the context of a route.` ) } } - const workUnitStore = workUnitAsyncStorage.getStore() - - if (!workUnitStore) { - throw new Error( - `Route ${workStore.route} used ${apiName} in Pages Router. This API is only available within App Router.` - ) - } - switch (workUnitStore.type) { case 'unstable-cache': case 'cache': { diff --git a/test/e2e/app-dir/app-root-params-getters/fixtures/simple/app/[lang]/[locale]/rerender-after-server-action/page.tsx b/test/e2e/app-dir/app-root-params-getters/fixtures/simple/app/[lang]/[locale]/rerender-after-server-action/page.tsx new file mode 100644 index 00000000000000..af65f6ed8a4055 --- /dev/null +++ b/test/e2e/app-dir/app-root-params-getters/fixtures/simple/app/[lang]/[locale]/rerender-after-server-action/page.tsx @@ -0,0 +1,37 @@ +import { lang, locale } from 'next/root-params' +import { connection } from 'next/server' +import { cookies } from 'next/headers' +import { Suspense } from 'react' + +export default async function Page() { + const currentLang = await lang() + const currentLocale = await locale() + return ( +
+
+ Root params are{' '} + + {currentLang} {currentLocale} + +
+ + + +
{ + 'use server' + // rerender the page and return it alongside the action result + const cookieStore = await cookies() + cookieStore.set('my-cookie', Date.now() + '') + }} + > + +
+
+ ) +} + +async function Timestamp() { + await connection() + return
{Date.now()}
+} diff --git a/test/e2e/app-dir/app-root-params-getters/simple.test.ts b/test/e2e/app-dir/app-root-params-getters/simple.test.ts index 6bd705fed8fb79..2c9d11374714dd 100644 --- a/test/e2e/app-dir/app-root-params-getters/simple.test.ts +++ b/test/e2e/app-dir/app-root-params-getters/simple.test.ts @@ -6,6 +6,23 @@ import { outdent } from 'outdent' import { createRequestTracker } from '../../../lib/e2e-utils/request-tracker' describe('app-root-param-getters - simple', () => { + let currentCliOutputIndex = 0 + beforeEach(() => { + resetCliOutput() + }) + + const getCliOutput = () => { + if (next.cliOutput.length < currentCliOutputIndex) { + // cliOutput shrank since we started the test, so something (like a `sandbox`) reset the logs + currentCliOutputIndex = 0 + } + return next.cliOutput.slice(currentCliOutputIndex) + } + + const resetCliOutput = () => { + currentCliOutputIndex = next.cliOutput.length + } + const { next, isNextDev, isTurbopack, isNextDeploy } = nextTestSetup({ files: join(__dirname, 'fixtures', 'simple'), }) @@ -109,12 +126,55 @@ describe('app-root-param-getters - simple', () => { ) expect(response.status()).toBe(500) if (!isNextDeploy) { - expect(next.cliOutput).toInclude( + expect(getCliOutput()).toInclude( "`import('next/root-params').lang()` was used inside a Server Action. This is not supported. Functions from 'next/root-params' can only be called in the context of a route." ) } }) + it('should not error when rerendering the page after a server action', async () => { + const params = { lang: 'en', locale: 'us' } + const browser = await next.browser( + `/${params.lang}/${params.locale}/rerender-after-server-action` + ) + expect(await browser.elementById('root-params').text()).toBe( + `${params.lang} ${params.locale}` + ) + const initialDate = await browser.elementById('timestamp') + + // Run a server action and rerender the page + const tracker = createRequestTracker(browser) + const [, response] = await tracker.captureResponse( + async () => { + await browser.elementByCss('button[type="submit"]').click() + }, + { + request: { + method: 'POST', + pathname: `/${params.lang}/${params.locale}/rerender-after-server-action`, + }, + } + ) + // We're using lang() outside of an action, so we should see no errors + expect(response.status()).toBe(200) + if (!isNextDeploy) { + expect(getCliOutput()).not.toInclude( + "`import('next/root-params').lang()` was used inside a Server Action. This is not supported. Functions from 'next/root-params' can only be called in the context of a route." + ) + } + + await retry(async () => { + // The page should've been rerendered because of the cookie update + const updatedDate = await browser.elementById('timestamp') + expect(initialDate).not.toEqual(updatedDate) + }) + + // It should still display correct root params + expect(await browser.elementById('root-params').text()).toBe( + `${params.lang} ${params.locale}` + ) + }) + // TODO(root-params): add support for route handlers it('should error when used in a route handler (until we implement it)', async () => { const params = { lang: 'en', locale: 'us' } @@ -123,7 +183,7 @@ describe('app-root-param-getters - simple', () => { ) expect(response.status).toBe(500) if (!isNextDeploy) { - expect(next.cliOutput).toInclude( + expect(getCliOutput()).toInclude( "Route /[lang]/[locale]/route-handler used `import('next/root-params').lang()` inside a Route Handler. Support for this API in Route Handlers is planned for a future version of Next.js." ) }