From 909a9c9b18e31857259dc3d0e31cd75a34bccaa9 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 16 Mar 2026 09:49:12 +0100 Subject: [PATCH 1/6] [test] Improve actions-tree-shaking tests (#91326) 1. Don't build twice for edge, patch source before building 2. Use the names for snapshots, as opposed to just a count that is impossible to debug. --- .../actions-tree-shaking/_testing/utils.ts | 94 +++++++++++-------- .../actions-tree-shaking/basic/basic.test.ts | 44 ++++----- .../mixed-module-actions.test.ts | 37 ++++---- .../reexport/reexport.test.ts | 68 +++++++++----- .../shared-module-actions.test.ts | 52 +++++----- .../shared-module-actions/tsconfig.json | 30 ------ .../use-effect-actions.test.ts | 26 ++--- 7 files changed, 185 insertions(+), 166 deletions(-) delete mode 100644 test/production/app-dir/actions-tree-shaking/shared-module-actions/tsconfig.json diff --git a/test/production/app-dir/actions-tree-shaking/_testing/utils.ts b/test/production/app-dir/actions-tree-shaking/_testing/utils.ts index 56f6fd7a49d98c..fd49219fcd2b4c 100644 --- a/test/production/app-dir/actions-tree-shaking/_testing/utils.ts +++ b/test/production/app-dir/actions-tree-shaking/_testing/utils.ts @@ -1,10 +1,27 @@ -// @ts-ignore avoid ts errors during manual testing -import { type NextInstance } from 'e2e-utils' +import { nextTestSetup, type NextInstance } from 'e2e-utils' + +// This is from 'next/dist/build/webpack/plugins/flight-client-entry-plugin', but unfortunately +// Typescript breaks when importing it directly. +type Actions = { + [actionId: string]: { + exportedName?: string + filename?: string + workers: { + [name: string]: { + moduleId: string | number + async: boolean + } + } + layer: { + [name: string]: string + } + } +} async function getActionsMappingByRuntime( next: NextInstance, runtime: 'node' | 'edge' -) { +): Promise { const manifest = JSON.parse( await next.readFile('.next/server/server-reference-manifest.json') ) @@ -12,60 +29,61 @@ async function getActionsMappingByRuntime( return manifest[runtime] } -export function markLayoutAsEdge(next: NextInstance) { - beforeAll(async () => { - await next.stop() - const layoutContent = await next.readFile('app/layout.js') - await next.patchFile( - 'app/layout.js', - layoutContent + `\nexport const runtime = 'edge'` - ) - await next.start() +export function nextTestSetupActionTreeShaking(opts) { + let result = nextTestSetup({ + ...opts, + skipStart: !!process.env.TEST_EDGE, }) -} -/* -{ - [route path]: { [layer]: Set ] -} -*/ -type ActionsMappingOfRuntime = { - [actionId: string]: { - workers: { - [route: string]: string - } - layer: { - [route: string]: string - } + if (process.env.TEST_EDGE) { + beforeAll(async () => { + const layoutContent = await result.next.readFile('app/layout.js') + await result.next.patchFile( + 'app/layout.js', + layoutContent + `\nexport const runtime = 'edge'` + ) + await result.next.start() + }) } + + return result } + type ActionState = { [route: string]: { - [layer: string]: number + [layer: string]: string[] } } -function getActionsRoutesState( - actionsMappingOfRuntime: ActionsMappingOfRuntime -): ActionState { +function getActionsRoutesState(actionsMappingOfRuntime: Actions): ActionState { const state: ActionState = {} - Object.keys(actionsMappingOfRuntime).forEach((actionId) => { + for (const actionId in actionsMappingOfRuntime) { const action = actionsMappingOfRuntime[actionId] - const routePaths = Object.keys(action.workers) - - routePaths.forEach((routePath) => { + for (const routePath in action.workers) { if (!state[routePath]) { state[routePath] = {} } const layer = action.layer[routePath] if (!state[routePath][layer]) { - state[routePath][layer] = 0 + state[routePath][layer] = [] } - state[routePath][layer]++ - }) - }) + // Normalize when NEXT_SKIP_ISOLATE=1 + const filename = action.filename.startsWith('test/tmp/next-test-') + ? action.filename.slice( + action.filename.indexOf('/', 'test/tmp/next-test-'.length) + 1 + ) + : action.filename + state[routePath][layer].push(`${filename}#${action.exportedName}`) + } + } + + for (const layer of Object.values(state)) { + for (const list of Object.values(layer)) { + list.sort() + } + } return state } diff --git a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts index 64811245520124..ea7303a2023a7c 100644 --- a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts +++ b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts @@ -1,37 +1,39 @@ -import { nextTestSetup } from 'e2e-utils' import { + nextTestSetupActionTreeShaking, getActionsRoutesStateByRuntime, - markLayoutAsEdge, } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action ;(process.env.IS_TURBOPACK_TEST ? describe : describe.skip)( 'actions-tree-shaking - basic', () => { - const { next } = nextTestSetup({ + const { next } = nextTestSetupActionTreeShaking({ files: __dirname, }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } - it('should not have the unused action in the manifest', async () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - - expect(actionsRoutesState).toMatchObject({ - // only one server layer action - 'app/server/page': { - rsc: 3, - }, - // only one browser layer action - 'app/client/page': { - 'action-browser': 1, - }, - 'app/inline/page': { - rsc: 1, - }, - }) + expect(actionsRoutesState).toMatchInlineSnapshot(` + { + "app/client/page": { + "action-browser": [ + "app/actions.js#clientComponentAction", + ], + }, + "app/inline/page": { + "rsc": [ + "app/inline/page.js#$$RSC_SERVER_ACTION_0", + ], + }, + "app/server/page": { + "rsc": [ + "app/actions.js#clientComponentAction", + "app/actions.js#serverComponentAction", + "app/actions.js#unusedExportedAction", + ], + }, + } + `) }) } ) diff --git a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts index f31661d7907a2b..ce333cf8c46e56 100644 --- a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts @@ -1,33 +1,36 @@ -import { nextTestSetup } from 'e2e-utils' import { + nextTestSetupActionTreeShaking, getActionsRoutesStateByRuntime, - markLayoutAsEdge, } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action ;(process.env.IS_TURBOPACK_TEST ? describe : describe.skip)( 'actions-tree-shaking - mixed-module-actions', () => { - const { next } = nextTestSetup({ + const { next } = nextTestSetupActionTreeShaking({ files: __dirname, }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } - it('should not do tree shake for cjs module when import server actions', async () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - - expect(actionsRoutesState).toMatchObject({ - 'app/mixed-module/esm/page': { - rsc: 3, - }, - // CJS import is not able to tree shake, so it will include all actions - 'app/mixed-module/cjs/page': { - rsc: 3, - }, - }) + expect(actionsRoutesState).toMatchInlineSnapshot(` + { + "app/mixed-module/cjs/page": { + "rsc": [ + "app/mixed-module/cjs/actions.js#cjsModuleTypeAction", + "app/mixed-module/cjs/actions.js#esmModuleTypeAction", + "app/mixed-module/cjs/actions.js#unusedModuleTypeAction1", + ], + }, + "app/mixed-module/esm/page": { + "rsc": [ + "app/mixed-module/esm/actions.js#cjsModuleTypeAction", + "app/mixed-module/esm/actions.js#esmModuleTypeAction", + "app/mixed-module/esm/actions.js#unusedModuleTypeAction1", + ], + }, + } + `) }) } ) diff --git a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts index 528c9cb60ba607..707511b1c5ac52 100644 --- a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts +++ b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts @@ -1,7 +1,6 @@ -import { nextTestSetup } from 'e2e-utils' import { + nextTestSetupActionTreeShaking, getActionsRoutesStateByRuntime, - markLayoutAsEdge, } from '../_testing/utils' import { retry } from 'next-test-utils' @@ -9,36 +8,55 @@ import { retry } from 'next-test-utils' ;(process.env.IS_TURBOPACK_TEST ? describe : describe.skip)( 'actions-tree-shaking - reexport', () => { - const { next } = nextTestSetup({ + const { next } = nextTestSetupActionTreeShaking({ files: __dirname, skipDeployment: true, }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } - it('should not tree-shake namespace exports the manifest', async () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - expect(actionsRoutesState).toMatchObject({ - 'app/namespace-reexport/server/page': { - // Turbopack does not tree-shake server side chunks - rsc: process.env.IS_TURBOPACK_TEST ? 3 : 1, - }, - 'app/namespace-reexport/client/page': { - 'action-browser': 1, - }, - // We're not able to tree-shake these re-exports here in webpack mode - 'app/named-reexport/server/page': { - // Turbopack supports tree-shaking these re-exports - rsc: 3, - }, - 'app/named-reexport/client/page': { - // Turbopack supports tree-shaking these re-exports - 'action-browser': process.env.IS_TURBOPACK_TEST ? 1 : 3, - }, - }) + expect(actionsRoutesState).toMatchInlineSnapshot(` + { + "app/named-reexport/client/page": { + "action-browser": [ + "app/named-reexport/client/actions.js#sharedClientLayerAction", + ], + }, + "app/named-reexport/server/page": { + "rsc": [ + "app/named-reexport/server/actions.js#sharedServerLayerAction", + "app/named-reexport/server/actions.js#unusedServerLayerAction1", + "app/named-reexport/server/actions.js#unusedServerLayerAction2", + ], + }, + "app/namespace-reexport-2/client/page": { + "action-browser": [ + "app/namespace-reexport-2/actions/action-modules.js#action", + "app/namespace-reexport-2/nested.js#getFoo", + ], + }, + "app/namespace-reexport-2/server/page": { + "rsc": [ + "app/namespace-reexport-2/actions/action-modules.js#action", + "app/namespace-reexport-2/nested.js#foo", + "app/namespace-reexport-2/nested.js#getFoo", + ], + }, + "app/namespace-reexport/client/page": { + "action-browser": [ + "app/namespace-reexport/client/actions.js#sharedClientLayerAction", + ], + }, + "app/namespace-reexport/server/page": { + "rsc": [ + "app/namespace-reexport/server/actions.js#sharedServerLayerAction", + "app/namespace-reexport/server/actions.js#unusedServerLayerAction1", + "app/namespace-reexport/server/actions.js#unusedServerLayerAction2", + ], + }, + } + `) }) it('should keep all the action exports for namespace export case on client layer', async () => { diff --git a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts index eaadac04f29255..21ba4dc9e086a2 100644 --- a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts @@ -1,38 +1,46 @@ -import { nextTestSetup } from 'e2e-utils' import { + nextTestSetupActionTreeShaking, getActionsRoutesStateByRuntime, - markLayoutAsEdge, } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action ;(process.env.IS_TURBOPACK_TEST ? describe : describe.skip)( 'actions-tree-shaking - shared-module-actions', () => { - const { next } = nextTestSetup({ + const { next } = nextTestSetupActionTreeShaking({ files: __dirname, }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } - it('should not have the unused action in the manifest', async () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - - expect(actionsRoutesState).toMatchObject({ - 'app/server/one/page': { - rsc: 3, - }, - 'app/server/two/page': { - rsc: 3, - }, - 'app/client/one/page': { - 'action-browser': 1, - }, - 'app/client/two/page': { - 'action-browser': 1, - }, - }) + expect(actionsRoutesState).toMatchInlineSnapshot(` + { + "app/client/one/page": { + "action-browser": [ + "app/client/actions.js#sharedClientLayerAction", + ], + }, + "app/client/two/page": { + "action-browser": [ + "app/client/actions.js#sharedClientLayerAction", + ], + }, + "app/server/one/page": { + "rsc": [ + "app/server/actions.js#sharedServerLayerAction", + "app/server/actions.js#unusedServerLayerAction1", + "app/server/actions.js#unusedServerLayerAction2", + ], + }, + "app/server/two/page": { + "rsc": [ + "app/server/actions.js#sharedServerLayerAction", + "app/server/actions.js#unusedServerLayerAction1", + "app/server/actions.js#unusedServerLayerAction2", + ], + }, + } + `) }) } ) diff --git a/test/production/app-dir/actions-tree-shaking/shared-module-actions/tsconfig.json b/test/production/app-dir/actions-tree-shaking/shared-module-actions/tsconfig.json deleted file mode 100644 index d3b92e4edca0c8..00000000000000 --- a/test/production/app-dir/actions-tree-shaking/shared-module-actions/tsconfig.json +++ /dev/null @@ -1,30 +0,0 @@ -{ - "compilerOptions": { - "target": "ES2017", - "lib": ["dom", "dom.iterable", "esnext"], - "allowJs": true, - "skipLibCheck": true, - "strict": false, - "noEmit": true, - "incremental": true, - "module": "esnext", - "esModuleInterop": true, - "moduleResolution": "node", - "resolveJsonModule": true, - "isolatedModules": true, - "jsx": "react-jsx", - "plugins": [ - { - "name": "next" - } - ] - }, - "include": [ - "next-env.d.ts", - ".next/types/**/*.ts", - "**/*.ts", - "**/*.tsx", - ".next/dev/types/**/*.ts" - ], - "exclude": ["node_modules", "**/*.test.ts", "**/*.test.tsx"] -} diff --git a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts index 9bf34ef7b669db..ef87213ce185d4 100644 --- a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts @@ -1,25 +1,25 @@ -import { nextTestSetup } from 'e2e-utils' import { + nextTestSetupActionTreeShaking, getActionsRoutesStateByRuntime, - markLayoutAsEdge, } from '../_testing/utils' describe('actions-tree-shaking - use-effect-actions', () => { - const { next } = nextTestSetup({ + const { next } = nextTestSetupActionTreeShaking({ files: __dirname, }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } - it('should not tree shake the used action under useEffect', async () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - - expect(actionsRoutesState).toMatchObject({ - 'app/mixed/page': { - 'action-browser': 3, - }, - }) + expect(actionsRoutesState).toMatchInlineSnapshot(` + { + "app/mixed/page": { + "action-browser": [ + "app/mixed/actions.ts#action1", + "app/mixed/actions.ts#action2", + "app/mixed/actions.ts#action3", + ], + }, + } + `) }) }) From 1ab534927b5e617454d34690861661411f27d6a5 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 16 Mar 2026 11:22:35 +0100 Subject: [PATCH 2/6] Move database compaction from write batch commit to backend idle loop (#91258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What? Moves TurboPersistence database compaction out of the write batch commit path and into the backend's idle loop, where it can run repeatedly (up to 10 times) while the system is idle. ## Why? Previously, compaction was kicked off as a background task after each write batch commit, with the next write batch blocking until the previous compaction finished. This had several drawbacks: - Compaction was tightly coupled to the write path, adding latency to write batches that had to wait for the previous compaction to complete before starting. - Only a single compaction pass ran per write batch, which may not be sufficient to fully compact the database (e.g., after many small writes create numerous segments). - The compaction used `spawn` and a `JoinHandle` stored in a `Mutex`, adding complexity to the write batch and shutdown logic. By moving compaction to the idle loop, we: - Decouple compaction from writes, so write batches are never blocked by ongoing compaction. - Allow multiple compaction passes (up to 10) while idle, enabling the database to converge to a more compact state. - Simplify the write batch code by removing the `compact_join_handle` machinery (`Mutex`, `JoinHandle`, `spawn`). - Respect idle-end signals — compaction stops immediately when new work arrives. ## How? 1. **Added `compact()` method** to the `KeyValueDatabase` trait and `BackingStorageSealed` trait, with a default no-op implementation returning `Ok(false)`. Returns `Ok(true)` when compaction actually merged files, `Ok(false)` when there was nothing to compact. 2. **Implemented `compact()` for `TurboKeyValueDatabase`** — calls the existing `do_compact` helper synchronously, skipping compaction for short sessions or empty databases. Changed `do_compact` return type from `Result<()>` to `Result` to signal whether work was done. 3. **Wired `compact()` through `KvBackingStorage`** and the `Either`-based `BackingStorageSealed` impl. 4. **Added compaction loop in the backend idle handler** (`backend/mod.rs`) — after a successful snapshot commit, runs up to 10 compaction passes, checking for idle-end between each pass and stopping early if no more compaction is needed or idle ends. 5. **Removed from `TurboWriteBatch`**: the `compact_join_handle` field, the `Mutex>`, the background `spawn` call in `commit()`, and the blocking join in `write_batch()` and `shutdown()`. The `shutdown()` path retains its own compaction call for final compaction on exit. --------- Co-authored-by: Tobias Koppers Co-authored-by: Claude --- .../turbo-tasks-backend/src/backend/mod.rs | 26 +++++++++ .../src/backing_storage.rs | 8 +++ .../src/database/key_value_database.rs | 8 +++ .../src/database/turbo/mod.rs | 58 ++++++------------- .../src/kv_backing_storage.rs | 4 ++ 5 files changed, 64 insertions(+), 40 deletions(-) diff --git a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs index b6d328497bb2b7..bde31f5d03d740 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backend/mod.rs @@ -2797,6 +2797,32 @@ impl TurboTasksBackendInner { let snapshot = this.snapshot_and_persist(None, reason, turbo_tasks); if let Some((snapshot_start, new_data)) = snapshot { last_snapshot = snapshot_start; + + // Compact while idle (up to limit), regardless of + // whether the snapshot had new data. + const MAX_IDLE_COMPACTION_PASSES: usize = 10; + for _ in 0..MAX_IDLE_COMPACTION_PASSES { + let idle_ended = tokio::select! { + biased; + _ = &mut idle_end_listener => { + idle_end_listener = self.idle_end_event.listen(); + true + }, + _ = std::future::ready(()) => false, + }; + if idle_ended { + break; + } + match self.backing_storage.compact() { + Ok(true) => {} + Ok(false) => break, + Err(err) => { + eprintln!("Compaction failed: {err:?}"); + break; + } + } + } + if !new_data { fresh_idle = false; continue; diff --git a/turbopack/crates/turbo-tasks-backend/src/backing_storage.rs b/turbopack/crates/turbo-tasks-backend/src/backing_storage.rs index 61297a0f559c3b..8c200a108f575d 100644 --- a/turbopack/crates/turbo-tasks-backend/src/backing_storage.rs +++ b/turbopack/crates/turbo-tasks-backend/src/backing_storage.rs @@ -87,6 +87,10 @@ pub trait BackingStorageSealed: 'static + Send + Sync { category: SpecificTaskDataCategory, ) -> Result>; + fn compact(&self) -> Result { + Ok(false) + } + fn shutdown(&self) -> Result<()> { Ok(()) } @@ -152,6 +156,10 @@ where either::for_both!(self, this => this.batch_lookup_data(task_ids, category)) } + fn compact(&self) -> Result { + either::for_both!(self, this => this.compact()) + } + fn shutdown(&self) -> Result<()> { either::for_both!(self, this => this.shutdown()) } diff --git a/turbopack/crates/turbo-tasks-backend/src/database/key_value_database.rs b/turbopack/crates/turbo-tasks-backend/src/database/key_value_database.rs index 9c11973088d164..20f61daca93a3a 100644 --- a/turbopack/crates/turbo-tasks-backend/src/database/key_value_database.rs +++ b/turbopack/crates/turbo-tasks-backend/src/database/key_value_database.rs @@ -82,6 +82,14 @@ pub trait KeyValueDatabase { // this is an optional performance hint to the database } + /// Triggers compaction of the database. + /// + /// Returns `Ok(true)` if compaction actually merged files, `Ok(false)` if there was nothing + /// to compact. The default implementation is a no-op. + fn compact(&self) -> Result { + Ok(false) + } + fn shutdown(&self) -> Result<()> { Ok(()) } diff --git a/turbopack/crates/turbo-tasks-backend/src/database/turbo/mod.rs b/turbopack/crates/turbo-tasks-backend/src/database/turbo/mod.rs index c1b9345cd50ef5..1fbb24795eed2e 100644 --- a/turbopack/crates/turbo-tasks-backend/src/database/turbo/mod.rs +++ b/turbopack/crates/turbo-tasks-backend/src/database/turbo/mod.rs @@ -7,15 +7,13 @@ use std::{ }; use anyhow::{Ok, Result}; -use parking_lot::Mutex; use smallvec::SmallVec; use turbo_persistence::{ ArcBytes, CompactConfig, DbConfig, KeyBase, StoreKey, TurboPersistence, ValueBuffer, }; use turbo_tasks::{ - JoinHandle, message_queue::{TimingEvent, TraceEvent}, - spawn, turbo_tasks, + turbo_tasks, }; use crate::database::{ @@ -29,6 +27,8 @@ mod parallel_scheduler; /// Number of key families, see KeySpace enum for their numbers. const FAMILIES: usize = 4; +const COMPACTION_MESSAGE: &str = "Finished filesystem cache database compaction"; + const MB: u64 = 1024 * 1024; const COMPACT_CONFIG: CompactConfig = CompactConfig { min_merge_count: 3, @@ -42,7 +42,6 @@ const COMPACT_CONFIG: CompactConfig = CompactConfig { pub struct TurboKeyValueDatabase { db: Arc>, - compact_join_handle: Mutex>>>, is_ci: bool, is_short_session: bool, is_fresh: bool, @@ -61,7 +60,6 @@ impl TurboKeyValueDatabase { let db = Arc::new(TurboPersistence::open_with_config(versioned_path, CONFIG)?); Ok(Self { db: db.clone(), - compact_join_handle: Mutex::new(None), is_ci, is_short_session, is_fresh: db.is_empty(), @@ -105,41 +103,37 @@ impl KeyValueDatabase for TurboKeyValueDatabase { Self: 'l; fn write_batch(&self) -> Result> { - // Wait for the compaction to finish - if let Some(join_handle) = self.compact_join_handle.lock().take() { - join_handle.join()?; - } - // Start a new write batch Ok(TurboWriteBatch { batch: self.db.write_batch()?, db: &self.db, - compact_join_handle: (!self.is_short_session && !self.db.is_empty()) - .then_some(&self.compact_join_handle), }) } fn prevent_writes(&self) {} - fn shutdown(&self) -> Result<()> { - // Wait for the compaction to finish - if let Some(join_handle) = self.compact_join_handle.lock().take() { - join_handle.join()?; + fn compact(&self) -> Result { + if self.is_short_session || self.db.is_empty() { + return Ok(false); } + do_compact( + &self.db, + COMPACTION_MESSAGE, + available_parallelism().map_or(4, |c| max(4, c.get() / 2)), + ) + } + + fn shutdown(&self) -> Result<()> { // Compact the database on shutdown // (Avoid compacting a fresh database since we don't have any usage info yet) if !self.is_fresh { if self.is_ci { // Fully compact in CI to reduce cache size - do_compact( - &self.db, - "Finished filesystem cache database compaction", - usize::MAX, - )?; + do_compact(&self.db, COMPACTION_MESSAGE, usize::MAX)?; } else { // Compact with a reasonable limit in non-CI environments do_compact( &self.db, - "Finished filesystem cache database compaction", + COMPACTION_MESSAGE, available_parallelism().map_or(4, |c| max(4, c.get())), )?; } @@ -153,7 +147,7 @@ fn do_compact( db: &TurboPersistence, message: &'static str, max_merge_segment_count: usize, -) -> Result<()> { +) -> Result { let start = Instant::now(); // SystemTime for wall-clock timestamps in trace events (Instant has no // defined epoch so it can't be used for cross-process trace correlation). @@ -182,14 +176,13 @@ fn do_compact( vec![], ))); } - Ok(()) + Ok(ran) } pub struct TurboWriteBatch<'a> { batch: turbo_persistence::WriteBatch, TurboTasksParallelScheduler, FAMILIES>, db: &'a Arc>, - compact_join_handle: Option<&'a Mutex>>>>, } impl<'a> ConcurrentWriteBatch<'a> for TurboWriteBatch<'a> { @@ -207,22 +200,7 @@ impl<'a> ConcurrentWriteBatch<'a> for TurboWriteBatch<'a> { } fn commit(self) -> Result<()> { - // Commit the write batch self.db.commit_write_batch(self.batch)?; - - if let Some(compact_join_handle) = self.compact_join_handle { - // Start a new compaction in the background - let db = self.db.clone(); - let handle = spawn(async move { - do_compact( - &db, - "Finished filesystem cache database compaction", - available_parallelism().map_or(4, |c| max(4, c.get() / 2)), - ) - }); - compact_join_handle.lock().replace(handle); - } - Ok(()) } diff --git a/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs b/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs index b27e3766ccdfb1..dac8acf2319ab1 100644 --- a/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs +++ b/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs @@ -375,6 +375,10 @@ impl BackingStorageSealed .collect::>>() } + fn compact(&self) -> Result { + self.inner.database.compact() + } + fn shutdown(&self) -> Result<()> { self.inner.database.shutdown() } From d9751eaa14fc764aadd3c58f2373998e567192bd Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 16 Mar 2026 12:02:31 +0100 Subject: [PATCH 3/6] Turbopack: use ChunkGroupEntry::Shared, part 2 (#91279) Followup to https://github.com/vercel/next.js/pull/90978, which fixed the `ChunkGroup`s passed to `.chunk_group()`, forgot to update the the module graph entry types, which didn't match up. The `ChunkGroupEntry::Shared(ResolvedVc::upcast(*server_component))` one was more involved: - We used to have `layout.js [app-rsc] (Next.js Server Component)` --ChunkingType::Shared--> `layout.js [app-rsc]` (the actual module) - But this meant that the actual chunk group was `Shared(server_component.await?.module)` (the inner one) - This wasn't possible to use for the module graph construction though, as the client reference transform needs to see the `layout.js [app-rsc] (Next.js Server Component)` module during the traversal, to infer the parent server component of the given subgraph. So instead, move to - `loader-tree in the template` - --ChunkingType::Shared--> - `layout.js [app-rsc] (Next.js Server Component)` - --ChunkingType::Parallel--> - `layout.js [app-rsc]` The reason for that `ChunkingType::Shared` is so that we can chunk arbitrary modules that are loaded by `ChunkGroup::Entry`. Meaning that we can do `chunk_group(Shared(root_layout))).concat(chunk_group(Shared(layout))).concat(chunk_group(Entry(page)))` and rely on turbotask caching to deduplicate layout chunking across pages. --- crates/next-api/src/app.rs | 12 +++---- crates/next-core/src/app_page_loader_tree.rs | 30 ++++++++++++----- crates/next-core/src/base_loader_tree.rs | 2 +- .../next_app/app_client_references_chunks.rs | 4 +-- .../src/next_dynamic/dynamic_module.rs | 30 +++++------------ .../src/next_server_component/mod.rs | 1 - .../server_component_module.rs | 22 ++++++------- .../server_component_reference.rs | 33 ------------------- 8 files changed, 46 insertions(+), 88 deletions(-) delete mode 100644 crates/next-core/src/next_server_component/server_component_reference.rs diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index e12629d5c9e005..110771b1a83984 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -930,9 +930,7 @@ impl AppProject { .take(server_component_entries.len().saturating_sub(1)) { let graph = SingleModuleGraph::new_with_entries_visited_intern( - // This should really be ChunkGroupEntry::Shared(module.await?.module), - // but that breaks everything for some reason. - vec![ChunkGroupEntry::Entry(vec![ResolvedVc::upcast(*module)])], + vec![ChunkGroupEntry::Shared(ResolvedVc::upcast(*module))], visited_modules, should_trace, should_read_binding_usage, @@ -1895,9 +1893,7 @@ impl AppEndpoint { async { let chunk_group = chunking_context.chunk_group( server_component.ident(), - ChunkGroup::Shared(ResolvedVc::upcast( - server_component.await?.module, - )), + ChunkGroup::Shared(ResolvedVc::upcast(server_component)), module_graph, current_chunk_group.await?.availability_info, ); @@ -2157,9 +2153,9 @@ impl Endpoint for AppEndpoint { .await?, ); - Ok(Vc::cell(vec![ChunkGroupEntry::Entry(vec![ + Ok(Vc::cell(vec![ChunkGroupEntry::Shared( server_actions_loader, - ])])) + )])) } #[turbo_tasks::function] diff --git a/crates/next-core/src/app_page_loader_tree.rs b/crates/next-core/src/app_page_loader_tree.rs index 20e187a2020dc4..77292e4ac9bd7a 100644 --- a/crates/next-core/src/app_page_loader_tree.rs +++ b/crates/next-core/src/app_page_loader_tree.rs @@ -187,9 +187,13 @@ impl AppPageLoaderTreeBuilder { // This should use the same importing mechanism as create_module_tuple_code, so that // the relative order of items is retained (which isn't the case // when mixing ESM imports and requires). - self.base - .imports - .push(format!("const {identifier} = require(\"{inner_module_id}\");").into()); + self.base.imports.push( + format!( + "const {identifier} = require(/*turbopackChunkingType: \ + shared*/\"{inner_module_id}\");" + ) + .into(), + ); let source = dynamic_image_metadata_source( *ResolvedVc::upcast(self.base.module_asset_context), @@ -234,9 +238,13 @@ impl AppPageLoaderTreeBuilder { // This should use the same importing mechanism as create_module_tuple_code, so that the // relative order of items is retained (which isn't the case when mixing ESM imports and // requires). - self.base - .imports - .push(format!("const {identifier} = require(\"{inner_module_id}\");").into()); + self.base.imports.push( + format!( + "const {identifier} = require(/*turbopackChunkingType: \ + shared*/\"{inner_module_id}\");" + ) + .into(), + ); let module = StructuredImageModuleType::create_module( Vc::upcast(FileSource::new(path.clone())), BlurPlaceholderMode::None, @@ -295,9 +303,13 @@ impl AppPageLoaderTreeBuilder { // This should use the same importing mechanism as create_module_tuple_code, so that the // relative order of items is retained (which isn't the case when mixing ESM imports and // requires). - self.base - .imports - .push(format!("const {identifier} = require(\"{inner_module_id}\");").into()); + self.base.imports.push( + format!( + "const {identifier} = require(/*turbopackChunkingType: \ + shared*/\"{inner_module_id}\");" + ) + .into(), + ); let module = self .base diff --git a/crates/next-core/src/base_loader_tree.rs b/crates/next-core/src/base_loader_tree.rs index dc786bcd1df72a..84b0c13cf0d945 100644 --- a/crates/next-core/src/base_loader_tree.rs +++ b/crates/next-core/src/base_loader_tree.rs @@ -99,7 +99,7 @@ impl BaseLoaderTreeBuilder { self.imports.push( formatdoc!( r#" - const {} = () => require("MODULE_{}"); + const {} = () => require(/*turbopackChunkingType: shared*/"MODULE_{}"); "#, identifier, i diff --git a/crates/next-core/src/next_app/app_client_references_chunks.rs b/crates/next-core/src/next_app/app_client_references_chunks.rs index 21b2e04dfffd93..182e352f16f588 100644 --- a/crates/next-core/src/next_app/app_client_references_chunks.rs +++ b/crates/next-core/src/next_app/app_client_references_chunks.rs @@ -178,9 +178,7 @@ pub async fn get_app_client_references_chunks( client_references_by_server_component.into_iter() { let parent_chunk_group = *chunk_group_info - .get_index_of(ChunkGroup::Shared(ResolvedVc::upcast( - server_component.await?.module, - ))) + .get_index_of(ChunkGroup::Shared(ResolvedVc::upcast(server_component))) .await?; let base_ident = server_component.ident(); diff --git a/crates/next-core/src/next_dynamic/dynamic_module.rs b/crates/next-core/src/next_dynamic/dynamic_module.rs index 8f551c958362c1..5d288c7b9f16a9 100644 --- a/crates/next-core/src/next_dynamic/dynamic_module.rs +++ b/crates/next-core/src/next_dynamic/dynamic_module.rs @@ -1,6 +1,6 @@ use anyhow::Result; use indoc::formatdoc; -use turbo_rcstr::{RcStr, rcstr}; +use turbo_rcstr::rcstr; use turbo_tasks::{ResolvedVc, Vc}; use turbopack_core::{ chunk::{AsyncModuleInfo, ChunkableModule, ChunkingContext, ModuleChunkItemIdExt}, @@ -36,20 +36,12 @@ impl NextDynamicEntryModule { } } -fn dynamic_ref_description() -> RcStr { - rcstr!("next/dynamic reference") -} - impl NextDynamicEntryModule { - async fn module_reference(&self) -> Result>> { - Ok(ResolvedVc::upcast( - SingleChunkableModuleReference::new( - Vc::upcast(*self.module), - dynamic_ref_description(), - ExportUsage::all(), - ) - .to_resolved() - .await?, + fn module_reference(&self) -> Vc> { + Vc::upcast(SingleChunkableModuleReference::new( + Vc::upcast(*self.module), + rcstr!("next/dynamic reference"), + ExportUsage::all(), )) } } @@ -70,8 +62,9 @@ impl Module for NextDynamicEntryModule { #[turbo_tasks::function] async fn references(&self) -> Result> { - Ok(Vc::cell(vec![self.module_reference().await?])) + Ok(Vc::cell(vec![self.module_reference().to_resolved().await?])) } + #[turbo_tasks::function] fn side_effects(self: Vc) -> Vc { // This just exports another import @@ -95,12 +88,7 @@ impl ChunkableModule for NextDynamicEntryModule { impl EcmascriptChunkPlaceable for NextDynamicEntryModule { #[turbo_tasks::function] fn get_exports(&self) -> Vc { - let module_reference: Vc> = - Vc::upcast(SingleChunkableModuleReference::new( - Vc::upcast(*self.module), - dynamic_ref_description(), - ExportUsage::all(), - )); + let module_reference = self.module_reference(); EsmExports::reexport_including_default(module_reference) } diff --git a/crates/next-core/src/next_server_component/mod.rs b/crates/next-core/src/next_server_component/mod.rs index cb7dd4ea23d506..08ab1174c6ad8d 100644 --- a/crates/next-core/src/next_server_component/mod.rs +++ b/crates/next-core/src/next_server_component/mod.rs @@ -1,5 +1,4 @@ pub mod server_component_module; -pub(crate) mod server_component_reference; pub(crate) mod server_component_transition; pub use server_component_transition::NextServerComponentTransition; diff --git a/crates/next-core/src/next_server_component/server_component_module.rs b/crates/next-core/src/next_server_component/server_component_module.rs index 21336c141019ee..67cef9e4505621 100644 --- a/crates/next-core/src/next_server_component/server_component_module.rs +++ b/crates/next-core/src/next_server_component/server_component_module.rs @@ -8,7 +8,8 @@ use turbopack_core::{ ident::AssetIdent, module::{Module, ModuleSideEffects}, module_graph::ModuleGraph, - reference::{ModuleReference, ModuleReferences}, + reference::{ModuleReference, ModuleReferences, SingleChunkableModuleReference}, + resolve::ExportUsage, source::OptionSource, }; use turbopack_ecmascript::{ @@ -21,8 +22,6 @@ use turbopack_ecmascript::{ utils::StringifyJs, }; -use super::server_component_reference::NextServerComponentModuleReference; - #[turbo_tasks::value(shared)] pub struct NextServerComponentModule { pub module: ResolvedVc>, @@ -62,11 +61,11 @@ impl NextServerComponentModule { } impl NextServerComponentModule { - async fn module_reference(&self) -> Result>> { - Ok(ResolvedVc::upcast( - NextServerComponentModuleReference::new(Vc::upcast(*self.module)) - .to_resolved() - .await?, + fn module_reference(&self) -> Vc> { + Vc::upcast(SingleChunkableModuleReference::new( + Vc::upcast(*self.module), + rcstr!("Next.js Server Component"), + ExportUsage::all(), )) } } @@ -87,8 +86,9 @@ impl Module for NextServerComponentModule { #[turbo_tasks::function] async fn references(&self) -> Result> { - Ok(Vc::cell(vec![self.module_reference().await?])) + Ok(Vc::cell(vec![self.module_reference().to_resolved().await?])) } + #[turbo_tasks::function] fn side_effects(self: Vc) -> Vc { // This just exports another import @@ -112,9 +112,7 @@ impl ChunkableModule for NextServerComponentModule { impl EcmascriptChunkPlaceable for NextServerComponentModule { #[turbo_tasks::function] fn get_exports(&self) -> Vc { - let module_reference: Vc> = Vc::upcast( - NextServerComponentModuleReference::new(Vc::upcast(*self.module)), - ); + let module_reference = self.module_reference(); EsmExports::reexport_including_default(module_reference) } diff --git a/crates/next-core/src/next_server_component/server_component_reference.rs b/crates/next-core/src/next_server_component/server_component_reference.rs deleted file mode 100644 index a4a91a2782c3d0..00000000000000 --- a/crates/next-core/src/next_server_component/server_component_reference.rs +++ /dev/null @@ -1,33 +0,0 @@ -use turbo_tasks::{ResolvedVc, ValueToString, Vc}; -use turbopack_core::{ - chunk::ChunkingType, module::Module, reference::ModuleReference, resolve::ModuleResolveResult, -}; - -#[turbo_tasks::value] -#[derive(ValueToString)] -#[value_to_string("Next.js Server Component {}", self.asset.ident())] -pub struct NextServerComponentModuleReference { - asset: ResolvedVc>, -} - -#[turbo_tasks::value_impl] -impl NextServerComponentModuleReference { - #[turbo_tasks::function] - pub fn new(asset: ResolvedVc>) -> Vc { - NextServerComponentModuleReference { asset }.cell() - } -} - -#[turbo_tasks::value_impl] -impl ModuleReference for NextServerComponentModuleReference { - #[turbo_tasks::function] - fn resolve_reference(&self) -> Vc { - *ModuleResolveResult::module(self.asset) - } - fn chunking_type(&self) -> Option { - Some(ChunkingType::Shared { - inherit_async: true, - merge_tag: None, - }) - } -} From 742b186670001882fbe56cd1898befa14133149c Mon Sep 17 00:00:00 2001 From: "cursor[bot]" <206951365+cursor[bot]@users.noreply.github.com> Date: Mon, 16 Mar 2026 12:28:43 +0100 Subject: [PATCH 4/6] Ready in X - prints wrong timing on dev server restart (#90874) ### What? Fixes the "Ready in" printout for the Next.js dev server after restarts. ### Why? When the dev server restarts (e.g., due to `next.config` changes), the "Ready in" time was incorrect, showing the total process uptime instead of the actual startup duration for the newly restarted server instance. This was because `NEXT_PRIVATE_START_TIME` was not reset. ### How? `process.env.NEXT_PRIVATE_START_TIME` is now updated to `Date.now().toString()` in `packages/next/src/cli/next-dev.ts` right before `startServer()` is re-called when handling a `RESTART_EXIT_CODE`. This ensures the "Ready in" time accurately reflects the startup duration from the point of restart. The tricky part has been writing a test, that both catches a regression, and won't be flaky on CI. After some back and forth, I think the Vercel bot suggestion is probably the simplest. --------- Co-authored-by: Cursor Agent Co-authored-by: Joseph Chamochumbi Co-authored-by: Vercel Co-authored-by: icyJoseph --- packages/next/src/cli/next-dev.ts | 4 ++ .../app-dir/watch-config-file/index.test.ts | 53 ++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts index 9b2d26a0003685..8178d20c736b61 100644 --- a/packages/next/src/cli/next-dev.ts +++ b/packages/next/src/cli/next-dev.ts @@ -391,6 +391,10 @@ const nextDev = async ( }) } + // Reset the start time so "Ready in X" reflects the restart + // duration, not time since the original process started. + process.env.NEXT_PRIVATE_START_TIME = Date.now().toString() + return startServer({ ...startServerOptions, port }) } // Call handler (e.g. upload telemetry). Don't try to send a signal to diff --git a/test/development/app-dir/watch-config-file/index.test.ts b/test/development/app-dir/watch-config-file/index.test.ts index c6ced0ab558e5e..913f9f035f6800 100644 --- a/test/development/app-dir/watch-config-file/index.test.ts +++ b/test/development/app-dir/watch-config-file/index.test.ts @@ -1,5 +1,5 @@ import { nextTestSetup } from 'e2e-utils' -import { check } from 'next-test-utils' +import { check, retry } from 'next-test-utils' import { join } from 'path' describe('app-dir watch-config-file', () => { @@ -34,4 +34,55 @@ describe('app-dir watch-config-file', () => { await check(() => next.fetch('/about').then((res) => res.status), 200) }) + + it('should show accurate Ready in duration after restart', async () => { + // No shared parser for "Ready in", handle all units + const toMs = (m: RegExpMatchArray) => { + const v = parseFloat(m[1]) + const u = m[2] + return u === 'min' ? v * 60_000 : u === 's' ? v * 1000 : v + } + + await retry(async () => { + expect(next.cliOutput).toMatch(/✓ Ready in /) + }) + + const initialMatch = next.cliOutput.match( + /✓ Ready in (\d+(?:\.\d+)?)(ms|s|min)/ + ) + expect(initialMatch).not.toBeNull() + + const outputBeforeRestart = next.cliOutput.length + + // Trigger restart, retried because the file watcher may need multiple writes + await check(async () => { + await next.patchFile( + 'next.config.js', + ` + const nextConfig = { + poweredByHeader: false, + } + module.exports = nextConfig` + ) + return next.cliOutput + }, /Found a change in next\.config\.js\. Restarting the server to apply the changes\.\.\./) + + // Restart duration should be comparable to initial startup + await retry( + async () => { + const postRestartOutput = next.cliOutput.slice(outputBeforeRestart) + const restartMatch = postRestartOutput.match( + /✓ Ready in (\d+(?:\.\d+)?)(ms|s|min)/ + ) + expect(restartMatch).not.toBeNull() + + const restartMs = toMs(restartMatch!) + // The restart should complete in well under 2 minutes. + // Before the fix, this would show the total process uptime (e.g., 84 min). + expect(restartMs).toBeLessThan(120_000) + }, + 30_000, + 1000 + ) + }) }) From 261922df82272dd44ec18f2042173b7797526fda Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 16 Mar 2026 13:41:29 +0100 Subject: [PATCH 5/6] Show generated code from loaders in parse error messages (#89898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What? When a webpack/turbopack loader produces broken code, error messages now display **both** the original source and the generated code with source map information, making it much easier to debug loader issues. ## Why? Previously, when loaders returned invalid code, error messages only showed the original source file (after source-map remapping). Users had no way to see what the loader actually generated, making it hard to diagnose why the code failed to parse. Showing both sides gives full context about what went wrong. ## How? ### Turbopack Core (`turbopack-core`) - **`Source::description()`** — New method on the `Source` trait providing human-readable descriptions of where code comes from. Implemented across all source types (`FileSource`, `VirtualSource`, `WebpackLoadersProcessedAsset`, `PostCssTransformedAsset`, etc.), producing chains like `"loaders [sass-loader] transform of file content of ./styles.scss"`. - **`AdditionalIssueSource`** — New struct to hold a labeled source location. The `Issue` trait gains an `additional_sources()` method so issues can expose supplementary code frames. - **`GeneratedCodeSource`** — A wrapper that strips `GenerateSourceMap` support from a source, ensuring the *generated* code is displayed as-is rather than being remapped back to the original. - **`IssueSource::to_generated_code_source()`** — Helper that detects sources implementing `GenerateSourceMap` and wraps them in `GeneratedCodeSource` for display. Used by `AnalyzeIssue` and `ParsingIssue` to automatically attach generated code frames. ### Error Formatting - **`turbopack-cli-utils`** — Renders additional sources in CLI issue output. - **`format-issue.ts`** — Renders additional sources in the browser error overlay. Extracted `formatSourceCodeFrame()` helper to deduplicate code-frame rendering between primary and additional sources. - Long-line truncation (e.g. minified CSS from SCSS) is handled natively by the Rust-based `codeFrameColumns` implementation. ### Type Definitions - Added `SourcePosition`, `IssueSource`, and `AdditionalIssueSource` interfaces to TypeScript types. - Updated `PlainSource` (added `file_path`), `PlainIssue` (added `additional_sources`), and NAPI bindings to pass the data through. ### Test Coverage - **E2e tests** (`test/e2e/webpack-loader-parse-error/`) with custom broken JS and CSS loaders, covering all 4 modes: - **Development (Turbopack)** — Verifies parse errors show both original and generated code via browser error overlay - **Development (Webpack)** — Verifies error overlay shows the parse error (webpack doesn't support additional sources) - **Production (Turbopack)** — Verifies build failure output with full error extraction and inline snapshots - **Production (Webpack)** — Verifies build failure output with inline snapshots - Updated `test/development/sass-error/` snapshot to include the new generated code frame for minified SCSS output. ### Example Output When a loader produces broken code, users now see: ``` ⨯ ./app/data.broken.js:3:1 Parsing ecmascript source code failed 1 | // This file will be processed by broken-js-loader 2 | // The loader will return invalid JavaScript with a source map > 3 | export default function Data() { | ^ 4 | return
original source content
5 | } 6 | Expected ' 3 | return
this is intentionally broken {{{ invalid jsx | ^ 4 | } 5 | Import trace: Server Component: ./app/data.broken.js ./app/page.js ``` --------- Co-authored-by: Claude Co-authored-by: Tobias Koppers Co-authored-by: Luke Sandberg --- .../next-core/src/next_image/source_asset.rs | 10 +- .../next-napi-bindings/src/next_api/utils.rs | 38 ++- .../next/src/build/swc/generated-native.d.ts | 8 + packages/next/src/build/swc/types.ts | 45 +-- .../next/src/shared/lib/turbopack/utils.ts | 25 +- test/development/sass-error/index.test.ts | 5 + .../app/css-page/page.js | 9 + .../app/css-page/styles.broken.css | 4 + .../app/data.broken.js | 5 + .../webpack-loader-parse-error/app/layout.js | 7 + .../webpack-loader-parse-error/app/page.js | 10 + .../broken-css-loader.js | 33 ++ .../broken-js-loader.js | 23 ++ .../webpack-loader-parse-error/next.config.js | 38 +++ .../webpack-loader-parse-error.test.ts | 299 ++++++++++++++++++ .../crates/turbopack-cli-utils/src/issue.rs | 24 ++ .../turbopack-core/src/data_uri_source.rs | 12 + .../crates/turbopack-core/src/file_source.rs | 5 + .../src/generated_code_source.rs | 46 +++ .../turbopack-core/src/issue/analyze.rs | 15 +- .../crates/turbopack-core/src/issue/mod.rs | 82 +++++ turbopack/crates/turbopack-core/src/lib.rs | 1 + turbopack/crates/turbopack-core/src/source.rs | 9 + .../turbopack-core/src/virtual_source.rs | 9 +- turbopack/crates/turbopack-css/src/process.rs | 12 +- .../turbopack-ecmascript/src/text/mod.rs | 6 + turbopack/crates/turbopack-env/src/asset.rs | 6 + turbopack/crates/turbopack-mdx/src/lib.rs | 6 + .../turbopack-node/src/transforms/postcss.rs | 11 + .../turbopack-node/src/transforms/webpack.rs | 39 ++- turbopack/crates/turbopack-wasm/src/source.rs | 9 + 31 files changed, 807 insertions(+), 44 deletions(-) create mode 100644 test/e2e/webpack-loader-parse-error/app/css-page/page.js create mode 100644 test/e2e/webpack-loader-parse-error/app/css-page/styles.broken.css create mode 100644 test/e2e/webpack-loader-parse-error/app/data.broken.js create mode 100644 test/e2e/webpack-loader-parse-error/app/layout.js create mode 100644 test/e2e/webpack-loader-parse-error/app/page.js create mode 100644 test/e2e/webpack-loader-parse-error/broken-css-loader.js create mode 100644 test/e2e/webpack-loader-parse-error/broken-js-loader.js create mode 100644 test/e2e/webpack-loader-parse-error/next.config.js create mode 100644 test/e2e/webpack-loader-parse-error/webpack-loader-parse-error.test.ts create mode 100644 turbopack/crates/turbopack-core/src/generated_code_source.rs diff --git a/crates/next-core/src/next_image/source_asset.rs b/crates/next-core/src/next_image/source_asset.rs index 84922db3a2d376..d6be7fe7b00bcc 100644 --- a/crates/next-core/src/next_image/source_asset.rs +++ b/crates/next-core/src/next_image/source_asset.rs @@ -1,8 +1,8 @@ use std::io::Write; use anyhow::{Result, bail}; -use turbo_rcstr::rcstr; -use turbo_tasks::{ResolvedVc, Vc}; +use turbo_rcstr::{RcStr, rcstr}; +use turbo_tasks::{ResolvedVc, ValueToString, Vc}; use turbo_tasks_fs::{FileContent, rope::RopeBuilder}; use turbopack_core::{ asset::{Asset, AssetContent}, @@ -47,6 +47,12 @@ impl Source for StructuredImageFileSource { .with_modifier(modifier) .rename_as(rcstr!("*.mjs")) } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let ident = self.image.ident().to_string().await?; + Ok(Vc::cell(format!("structured image of {}", ident).into())) + } } #[turbo_tasks::value_impl] diff --git a/crates/next-napi-bindings/src/next_api/utils.rs b/crates/next-napi-bindings/src/next_api/utils.rs index b04b419c12b160..75add6798cceb9 100644 --- a/crates/next-napi-bindings/src/next_api/utils.rs +++ b/crates/next-napi-bindings/src/next_api/utils.rs @@ -155,22 +155,19 @@ fn is_internal(file_path: &str) -> bool { RE.is_match(file_path) } -/// Renders a code frame for the issue's source location, if available. +/// Renders a code frame for a source location, if available. /// /// This avoids transferring the full source file content across the NAPI /// boundary just to call back into Rust for code frame rendering. /// /// Because this accesses the terminal size, this function call should not be cached (e.g. in /// turbo-tasks). -fn render_issue_code_frame(issue: &PlainIssue) -> Result> { - let Some(source) = issue.source.as_ref() else { - return Ok(None); - }; +fn render_source_code_frame(source: &PlainIssueSource, file_path: &str) -> Result> { let Some((start, end)) = source.range else { return Ok(None); }; - if is_internal(&issue.file_path) { + if is_internal(file_path) { return Ok(None); } @@ -210,6 +207,14 @@ fn render_issue_code_frame(issue: &PlainIssue) -> Result> { ) } +/// Renders a code frame for the issue's primary source location. +fn render_issue_code_frame(issue: &PlainIssue) -> Result> { + let Some(source) = issue.source.as_ref() else { + return Ok(None); + }; + render_source_code_frame(source, &issue.file_path) +} + #[napi(object)] pub struct NapiIssue { pub severity: String, @@ -219,6 +224,7 @@ pub struct NapiIssue { pub description: Option, pub detail: Option, pub source: Option, + pub additional_sources: Vec, pub documentation_link: String, pub import_traces: serde_json::Value, /// Pre-rendered code frame for the issue's source location, if available. @@ -226,6 +232,14 @@ pub struct NapiIssue { pub code_frame: Option, } +#[napi(object)] +pub struct NapiAdditionalIssueSource { + pub description: String, + pub source: NapiIssueSource, + /// Pre-rendered code frame for this additional source location, if available. + pub code_frame: Option, +} + impl From<&PlainIssue> for NapiIssue { fn from(issue: &PlainIssue) -> Self { Self { @@ -242,6 +256,16 @@ impl From<&PlainIssue> for NapiIssue { documentation_link: issue.documentation_link.to_string(), severity: issue.severity.as_str().to_string(), source: issue.source.as_ref().map(|source| source.into()), + additional_sources: issue + .additional_sources + .iter() + .map(|s| NapiAdditionalIssueSource { + description: s.description.to_string(), + code_frame: render_source_code_frame(&s.source, &s.source.asset.file_path) + .unwrap_or_default(), + source: (&s.source).into(), + }) + .collect(), title: serde_json::to_value(StyledStringSerialize::from(&issue.title)).unwrap(), import_traces: serde_json::to_value(&issue.import_traces).unwrap(), code_frame: render_issue_code_frame(issue).unwrap_or_default(), @@ -323,12 +347,14 @@ impl From<&(SourcePos, SourcePos)> for NapiIssueSourceRange { #[napi(object)] pub struct NapiSource { pub ident: String, + pub file_path: String, } impl From<&PlainSource> for NapiSource { fn from(source: &PlainSource) -> Self { Self { ident: source.ident.to_string(), + file_path: source.file_path.to_string(), } } } diff --git a/packages/next/src/build/swc/generated-native.d.ts b/packages/next/src/build/swc/generated-native.d.ts index 927d9bc8f8e08a..4ae6b55464ef85 100644 --- a/packages/next/src/build/swc/generated-native.d.ts +++ b/packages/next/src/build/swc/generated-native.d.ts @@ -519,6 +519,7 @@ export interface NapiIssue { description?: any detail?: any source?: NapiIssueSource + additionalSources: Array documentationLink: string importTraces: any /** @@ -527,6 +528,12 @@ export interface NapiIssue { */ codeFrame?: string } +export interface NapiAdditionalIssueSource { + description: string + source: NapiIssueSource + /** Pre-rendered code frame for this additional source location, if available. */ + codeFrame?: string +} export interface NapiIssueSource { source: NapiSource range?: NapiIssueSourceRange @@ -537,6 +544,7 @@ export interface NapiIssueSourceRange { } export interface NapiSource { ident: string + filePath: string } export interface NapiSourcePos { line: number diff --git a/packages/next/src/build/swc/types.ts b/packages/next/src/build/swc/types.ts index 8368783e9b6794..d4901aa1bc7747 100644 --- a/packages/next/src/build/swc/types.ts +++ b/packages/next/src/build/swc/types.ts @@ -112,6 +112,30 @@ export type StyledString = value: StyledString[] } +/** 0-indexed line and column position within a source file. */ +export interface SourcePosition { + line: number + column: number +} + +export interface IssueSource { + source: { + ident: string + filePath: string + } + range?: { + start: SourcePosition + end: SourcePosition + } +} + +export interface AdditionalIssueSource { + description: string + source: IssueSource + /** Pre-rendered code frame from the Rust NAPI layer */ + codeFrame?: string +} + export interface Issue { severity: string stage: string @@ -119,25 +143,8 @@ export interface Issue { title: StyledString description?: StyledString detail?: StyledString - source?: { - source: { - ident: string - } - range?: { - start: { - // 0-indexed - line: number - // 0-indexed - column: number - } - end: { - // 0-indexed - line: number - // 0-indexed - column: number - } - } - } + source?: IssueSource + additionalSources?: AdditionalIssueSource[] documentationLink: string importTraces?: PlainTraceItem[][] /** Pre-rendered code frame from the Rust NAPI layer */ diff --git a/packages/next/src/shared/lib/turbopack/utils.ts b/packages/next/src/shared/lib/turbopack/utils.ts index 8bdf14f7f3b445..5fd1ef1abd2d34 100644 --- a/packages/next/src/shared/lib/turbopack/utils.ts +++ b/packages/next/src/shared/lib/turbopack/utils.ts @@ -91,6 +91,13 @@ export function processIssues( } } +function formatFilePath(filePath: string): string { + return filePath + .replace('[project]/', './') + .replaceAll('/./', '/') + .replace('\\\\?\\', '') +} + export function formatIssue(issue: Issue) { const { filePath, title, description, detail, source, importTraces } = issue let { documentationLink } = issue @@ -107,10 +114,7 @@ export function formatIssue(issue: Issue) { documentationLink = 'https://nextjs.org/docs/messages/module-not-found' } - const formattedFilePath = filePath - .replace('[project]/', './') - .replaceAll('/./', '/') - .replace('\\\\?\\', '') + const formattedFilePath = formatFilePath(filePath) let message = '' @@ -149,6 +153,19 @@ export function formatIssue(issue: Issue) { message += renderStyledStringToErrorAnsi(detail) + '\n\n' } + // Render additional sources (e.g., generated code from a loader) + for (const additional of issue.additionalSources ?? []) { + if (additional.codeFrame) { + const additionalFilePath = formatFilePath( + additional.source.source.filePath + ) + const loc = additional.source.range + ? `:${additional.source.range.start.line + 1}:${additional.source.range.start.column + 1}` + : '' + message += `${additional.description}:\n${additionalFilePath}${loc}\n${additional.codeFrame.trimEnd()}\n\n` + } + } + if (importTraces?.length) { // This is the same logic as in turbopack/crates/turbopack-cli-utils/src/issue.rs // We end up with multiple traces when the file with the error is reachable from multiple diff --git a/test/development/sass-error/index.test.ts b/test/development/sass-error/index.test.ts index 38a644cc72bdcc..c94d1103bc1117 100644 --- a/test/development/sass-error/index.test.ts +++ b/test/development/sass-error/index.test.ts @@ -38,6 +38,11 @@ describe('app dir - css', () => { Pseudo-elements like '::before' or '::after' can't be followed by selectors like 'Ident("path")' + Generated code of PostCSS transform of loaders [next/dist/build/webpack/loaders/resolve-url-loader/index, next/dist/compiled/sass-loader] transform of file content of app/global.scss: + ./app/global.scss.css:1:884 + > 1 | ...ate(-50%, 0px)}input.defaultCheckbox::before path{fill:currentColor}input:checked.defaul... + | ^ + Import trace: Client Component Browser: ./app/global.scss.css [Client Component Browser] diff --git a/test/e2e/webpack-loader-parse-error/app/css-page/page.js b/test/e2e/webpack-loader-parse-error/app/css-page/page.js new file mode 100644 index 00000000000000..6f8593dbae6367 --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/app/css-page/page.js @@ -0,0 +1,9 @@ +import './styles.broken.css' + +export default function CssPage() { + return ( +
+

CSS Page

+
+ ) +} diff --git a/test/e2e/webpack-loader-parse-error/app/css-page/styles.broken.css b/test/e2e/webpack-loader-parse-error/app/css-page/styles.broken.css new file mode 100644 index 00000000000000..a7ebbc5b43816e --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/app/css-page/styles.broken.css @@ -0,0 +1,4 @@ +.page { + color: blue; + font-size: 16px; +} diff --git a/test/e2e/webpack-loader-parse-error/app/data.broken.js b/test/e2e/webpack-loader-parse-error/app/data.broken.js new file mode 100644 index 00000000000000..8e921e2faa692f --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/app/data.broken.js @@ -0,0 +1,5 @@ +// This file will be processed by broken-js-loader +// The loader will return invalid JavaScript with a source map +export default function Data() { + return
original source content
+} diff --git a/test/e2e/webpack-loader-parse-error/app/layout.js b/test/e2e/webpack-loader-parse-error/app/layout.js new file mode 100644 index 00000000000000..803f17d863c8ad --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/app/layout.js @@ -0,0 +1,7 @@ +export default function RootLayout({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/webpack-loader-parse-error/app/page.js b/test/e2e/webpack-loader-parse-error/app/page.js new file mode 100644 index 00000000000000..50dc75b5f40888 --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/app/page.js @@ -0,0 +1,10 @@ +import Broken from './data.broken.js' + +export default function Page() { + return ( +
+

Hello

+ +
+ ) +} diff --git a/test/e2e/webpack-loader-parse-error/broken-css-loader.js b/test/e2e/webpack-loader-parse-error/broken-css-loader.js new file mode 100644 index 00000000000000..cae072a0c09976 --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/broken-css-loader.js @@ -0,0 +1,33 @@ +// A webpack loader that returns invalid CSS with a source map. +// The source map points back to the original file, making it clear that the +// loader is responsible for the broken output (not the original source). +module.exports = function (source) { + // Generate a source map that maps generated lines back to the original source. + // The original file is valid CSS; the generated output is intentionally broken. + // + // Mapping (0-based line numbers): + // generated line 0 ("/* Generated by... */") -> original line 0 (".page {") + // generated line 1 (".page {") -> original line 0 (".page {") + // generated line 2 (" color: red") -> original line 1 (" color: blue;") + // generated line 3 (" @@@ ...") -> original line 2 (" font-size: 16px;") + // generated line 4 (" background: {{{...") -> original line 1 (" color: blue;") + // generated line 5 ("}") -> original line 3 ("}") + const sourceMap = { + version: 3, + sources: [this.resourcePath], + sourcesContent: [source], + mappings: 'AAAA;AAAA;AACE;AACA;AADA;AAEF', + names: [], + } + + // Return broken CSS that will fail to parse + const brokenCss = `/* Generated by broken-css-loader */ +.page { + color: red + @@@ THIS IS NOT VALID CSS @@@; + background: {{{ invalid +} +` + + this.callback(null, brokenCss, sourceMap) +} diff --git a/test/e2e/webpack-loader-parse-error/broken-js-loader.js b/test/e2e/webpack-loader-parse-error/broken-js-loader.js new file mode 100644 index 00000000000000..528cdd7297f515 --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/broken-js-loader.js @@ -0,0 +1,23 @@ +// A webpack loader that returns invalid JavaScript with a source map. +// The source map points back to the original file, but the generated +// code is intentionally broken (unparseable). +module.exports = function (source) { + // Generate a source map that maps back to the original source + const sourceMap = { + version: 3, + sources: [this.resourcePath], + sourcesContent: [source], + // identity mapping: each generated line maps to the same original line + mappings: 'AAAA;AACA;AACA;AACA', + names: [], + } + + // Return broken JavaScript that will fail to parse + const brokenCode = `// Generated by broken-js-loader +export default function Page() { + return
this is intentionally broken {{{ invalid jsx +} +` + + this.callback(null, brokenCode, sourceMap) +} diff --git a/test/e2e/webpack-loader-parse-error/next.config.js b/test/e2e/webpack-loader-parse-error/next.config.js new file mode 100644 index 00000000000000..faa02e404ee4ab --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/next.config.js @@ -0,0 +1,38 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + turbopack: { + rules: { + '*.broken.js': { + loaders: [ + { + loader: require.resolve('./broken-js-loader.js'), + }, + ], + }, + '*.broken.css': { + loaders: [ + { + loader: require.resolve('./broken-css-loader.js'), + }, + ], + }, + }, + }, + webpack: (config) => { + config.module.rules.push( + { + test: /\.broken\.js$/, + use: [require.resolve('./broken-js-loader.js')], + }, + { + test: /\.broken\.css$/, + use: [require.resolve('./broken-css-loader.js')], + } + ) + return config + }, +} + +module.exports = nextConfig diff --git a/test/e2e/webpack-loader-parse-error/webpack-loader-parse-error.test.ts b/test/e2e/webpack-loader-parse-error/webpack-loader-parse-error.test.ts new file mode 100644 index 00000000000000..3902e3dfb74f50 --- /dev/null +++ b/test/e2e/webpack-loader-parse-error/webpack-loader-parse-error.test.ts @@ -0,0 +1,299 @@ +import { nextTestSetup } from 'e2e-utils' +import { retry } from 'next-test-utils' +import stripAnsi from 'strip-ansi' + +/** + * Strips the dynamic test directory prefix from paths so snapshots are stable. + * Handles both absolute paths (/full/path/test/tmp/next-test-XXX/) and + * relative paths (./test/tmp/next-test-XXX/). + */ +function normalizePaths(output: string, testDir: string): string { + let result = output.replaceAll(testDir + '/', '') + const relMatch = testDir.match(/(test\/tmp\/next-test-[^/]+)/) + if (relMatch) { + result = result.replaceAll('./' + relMatch[1] + '/', './') + result = result.replaceAll(relMatch[1] + '/', '') + } + return result +} + +/** + * Extracts a single error block from CLI output by searching for a title string. + * + * Start boundary: the nearest `⨯` marker (dev) or file-path line (prod) + * before the title. + * End boundary: the first blank line after an "Import trace" section that + * isn't followed by continuation content (indented lines or file paths). + */ +function extractErrorBlock(output: string, errorTitle: string): string { + const titleIdx = output.indexOf(errorTitle) + if (titleIdx === -1) return '' + + const beforeTitle = output.substring(0, titleIdx) + + // In dev mode, errors start with a `⨯` marker on the same line as the file path. + // In prod mode, errors start with a bare file-path line above the title. + const markerIdx = beforeTitle.lastIndexOf('⨯ ') + let startIdx: number + if (markerIdx !== -1) { + startIdx = markerIdx + } else { + // Go back two lines: title is preceded by a file-path line (e.g. ./file:3:1) + const lines = beforeTitle.split('\n') + startIdx = + lines.length >= 2 + ? beforeTitle.length - + lines[lines.length - 1].length - + lines[lines.length - 2].length - + 1 + : 0 + } + + // Scan forward to find the end of the Import trace section + const block = output.substring(startIdx) + const lines = block.split('\n') + let inImportTrace = false + for (let i = 0; i < lines.length; i++) { + if (/^Import trace/.test(lines[i])) { + inImportTrace = true + } + // A blank line after the import trace ends the block, unless the next + // line is a continuation (indented or a file path) + if (inImportTrace && lines[i].trim() === '' && i > 0) { + const nextLine = lines[i + 1] + if ( + !nextLine || + (!nextLine.startsWith(' ') && !nextLine.startsWith('./')) + ) { + return lines.slice(0, i).join('\n') + } + } + } + return block.trimEnd() +} + +describe('webpack-loader-parse-error (development)', () => { + const { next, isTurbopack, isNextDev } = nextTestSetup({ + files: __dirname, + skipDeployment: true, + skipStart: true, + }) + + if (!isNextDev) { + it('skipped in production mode', () => {}) + return + } + + beforeAll(() => next.start()) + + it('should show parse error for JS loader that returns broken code', async () => { + const outputIndex = next.cliOutput.length + + // Fetch the page to trigger the error + await next.fetch('/') + + // Check that the CLI output contains the parse error + // Turbopack: "Parsing ecmascript source code failed" + // Webpack: "Syntax Error" + await retry(async () => { + expect(next.cliOutput).toMatch( + /Parsing ecmascript source code failed|Syntax Error/ + ) + }) + + if (isTurbopack) { + const output = normalizePaths( + stripAnsi(next.cliOutput.slice(outputIndex)), + next.testDir + ) + const errorBlock = extractErrorBlock( + output, + 'Parsing ecmascript source code failed' + ) + expect(errorBlock).toMatchInlineSnapshot(` + "⨯ ./app/data.broken.js:3:1 + Expected ' 3 | export default function Data() { + | ^ + 4 | return
original source content
+ 5 | } + 6 | + + Parsing ecmascript source code failed + + Generated code of loaders [broken-js-loader.js] transform of file content of app/data.broken.js: + ./app/data.broken.js:3:46 + 1 | // Generated by broken-js-loader + 2 | export default function Page() { + > 3 | return
this is intentionally broken {{{ invalid jsx + | ^ + 4 | } + 5 | + + Import trace: + Server Component: + ./app/data.broken.js + ./app/page.js" + `) + } + }) + + it('should show parse error for CSS loader that returns broken code', async () => { + const outputIndex = next.cliOutput.length + + // Fetch the page to trigger the CSS error + await next.fetch('/css-page') + + // Check that the CLI output contains the CSS parse error + // Turbopack: "Parsing CSS source code failed" + // Webpack: "Unknown word" + await retry(async () => { + expect(next.cliOutput).toMatch( + /Parsing CSS source code failed|Unknown word/ + ) + }) + + if (isTurbopack) { + const output = normalizePaths( + stripAnsi(next.cliOutput.slice(outputIndex)), + next.testDir + ) + const errorBlock = extractErrorBlock( + output, + 'Parsing CSS source code failed' + ) + expect(errorBlock).toMatchInlineSnapshot(` + "⨯ ./app/css-page/styles.broken.css:2:3 + Parsing CSS source code failed + 1 | .page { + > 2 | color: blue; + | ^ + 3 | font-size: 16px; + 4 | } + 5 | + + Unexpected token CurlyBracketBlock + + Generated code of PostCSS transform of loaders [broken-css-loader.js] transform of file content of app/css-page/styles.broken.css: + ./app/css-page/styles.broken.css:5:15 + 3 | color: red + 4 | @@@ THIS IS NOT VALID CSS @@@; + > 5 | background: {{{ invalid + | ^ + 6 | } + 7 | + + Import trace: + Client Component Browser: + ./app/css-page/styles.broken.css [Client Component Browser] + ./app/css-page/page.js [Server Component]" + `) + } + }) +}) + +describe('webpack-loader-parse-error (production)', () => { + const { next, isNextStart, isTurbopack } = nextTestSetup({ + files: __dirname, + skipDeployment: true, + skipStart: true, + }) + + if (!isNextStart) { + it('skipped in development mode', () => {}) + return + } + + it('should fail the build with parse errors from loaders', async () => { + await expect(next.start()).rejects.toThrow( + 'next build failed with code/signal 1' + ) + + const output = normalizePaths(stripAnsi(next.cliOutput), next.testDir) + + if (isTurbopack) { + const jsError = extractErrorBlock(output, "Expected ' 3 | export default function Data() { + | ^ + 4 | return
original source content
+ 5 | } + 6 | + + Parsing ecmascript source code failed + + Generated code of loaders [broken-js-loader.js] transform of file content of app/data.broken.js: + ./app/data.broken.js:3:46 + 1 | // Generated by broken-js-loader + 2 | export default function Page() { + > 3 | return
this is intentionally broken {{{ invalid jsx + | ^ + 4 | } + 5 | + + Import trace: + Server Component: + ./app/data.broken.js + ./app/page.js" + `) + + const cssError = extractErrorBlock( + output, + 'Parsing CSS source code failed' + ) + expect(cssError).toMatchInlineSnapshot(` + "./app/css-page/styles.broken.css:5:15 + Parsing CSS source code failed + 3 | color: red + 4 | @@@ THIS IS NOT VALID CSS @@@; + > 5 | background: {{{ invalid + | ^ + 6 | } + 7 | + + Unexpected token CurlyBracketBlock + + Generated code of PostCSS transform of loaders [broken-css-loader.js] transform of file content of app/css-page/styles.broken.css: + ./app/css-page/styles.broken.css:5:15 + 3 | color: red + 4 | @@@ THIS IS NOT VALID CSS @@@; + > 5 | background: {{{ invalid + | ^ + 6 | } + 7 | + + Import trace: + Client Component Browser: + ./app/css-page/styles.broken.css [Client Component Browser] + ./app/css-page/page.js [Server Component]" + `) + } else { + // Webpack stops at the first error (JS), showing the generated code + const jsError = extractErrorBlock(output, "Expected 'this is intentionally broken {{{ invalid jsx + : ^ + 4 | } + \`---- + + Caused by: + Syntax Error + + Import trace for requested module: + ./app/data.broken.js + ./app/page.js" + `) + } + }) +}) diff --git a/turbopack/crates/turbopack-cli-utils/src/issue.rs b/turbopack/crates/turbopack-cli-utils/src/issue.rs index 8614390ac3dbc4..b8ca545127151e 100644 --- a/turbopack/crates/turbopack-cli-utils/src/issue.rs +++ b/turbopack/crates/turbopack-cli-utils/src/issue.rs @@ -93,6 +93,30 @@ pub fn format_issue( writeln!(styled_issue, "{path}").unwrap(); } } + + // Render additional sources (e.g., generated code from a loader) + for additional in &plain_issue.additional_sources { + let desc = &additional.description; + let source = &additional.source; + match source.range { + Some((start, _)) => { + writeln!( + styled_issue, + "\n{}:\n{}:{}:{}", + desc, + source.asset.ident, + start.line + 1, + start.column + 1 + ) + .unwrap(); + } + None => { + writeln!(styled_issue, "\n{}:\n{}", desc, source.asset.ident).unwrap(); + } + } + format_source_content(source, &mut styled_issue); + } + let traces = &*plain_issue.import_traces; if !traces.is_empty() { /// Returns the leaf layer name, which is the first present layer name in the trace diff --git a/turbopack/crates/turbopack-core/src/data_uri_source.rs b/turbopack/crates/turbopack-core/src/data_uri_source.rs index fb07c67df0fc6b..053c0067a92808 100644 --- a/turbopack/crates/turbopack-core/src/data_uri_source.rs +++ b/turbopack/crates/turbopack-core/src/data_uri_source.rs @@ -40,6 +40,18 @@ impl DataUriSource { #[turbo_tasks::value_impl] impl Source for DataUriSource { + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let data = self.data.await?; + // Include a short prefix of the raw data for identification; data URIs + // can be very long so we cap it at 50 characters. + let prefix: String = data.chars().take(50).collect(); + let ellipsis = if data.len() > 50 { "..." } else { "" }; + Ok(Vc::cell( + format!("data URI content ({prefix}{ellipsis})").into(), + )) + } + #[turbo_tasks::function] async fn ident(&self) -> Result> { let content_type = self.media_type.split(";").next().unwrap().into(); diff --git a/turbopack/crates/turbopack-core/src/file_source.rs b/turbopack/crates/turbopack-core/src/file_source.rs index ef462cb662ddae..78d006f9391249 100644 --- a/turbopack/crates/turbopack-core/src/file_source.rs +++ b/turbopack/crates/turbopack-core/src/file_source.rs @@ -56,6 +56,11 @@ impl Source for FileSource { } ident } + + #[turbo_tasks::function] + fn description(&self) -> Vc { + Vc::cell(format!("file content of {}", self.path).into()) + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-core/src/generated_code_source.rs b/turbopack/crates/turbopack-core/src/generated_code_source.rs new file mode 100644 index 00000000000000..4a0ee6cf5dcb8f --- /dev/null +++ b/turbopack/crates/turbopack-core/src/generated_code_source.rs @@ -0,0 +1,46 @@ +use turbo_rcstr::RcStr; +use turbo_tasks::{ResolvedVc, Vc}; + +use crate::{ + asset::{Asset, AssetContent}, + ident::AssetIdent, + source::Source, +}; + +/// A source wrapping another source but stripping source map support. +/// Used to display generated code in error messages without triggering +/// source map remapping (since this type does NOT implement +/// `GenerateSourceMap`). +#[turbo_tasks::value] +pub struct GeneratedCodeSource { + source: ResolvedVc>, +} + +#[turbo_tasks::value_impl] +impl GeneratedCodeSource { + #[turbo_tasks::function] + pub fn new(source: ResolvedVc>) -> Vc { + Self { source }.cell() + } +} + +#[turbo_tasks::value_impl] +impl Source for GeneratedCodeSource { + #[turbo_tasks::function] + fn ident(&self) -> Vc { + self.source.ident() + } + + #[turbo_tasks::function] + fn description(&self) -> Vc { + self.source.description() + } +} + +#[turbo_tasks::value_impl] +impl Asset for GeneratedCodeSource { + #[turbo_tasks::function] + fn content(&self) -> Vc { + self.source.content() + } +} diff --git a/turbopack/crates/turbopack-core/src/issue/analyze.rs b/turbopack/crates/turbopack-core/src/issue/analyze.rs index 78ca967a17efef..5ff79cae0440b3 100644 --- a/turbopack/crates/turbopack-core/src/issue/analyze.rs +++ b/turbopack/crates/turbopack-core/src/issue/analyze.rs @@ -3,7 +3,10 @@ use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; use turbo_tasks_fs::FileSystemPath; -use super::{Issue, IssueSeverity, IssueSource, IssueStage, OptionStyledString, StyledString}; +use super::{ + AdditionalIssueSources, Issue, IssueSeverity, IssueSource, IssueStage, OptionStyledString, + StyledString, +}; use crate::{ident::AssetIdent, issue::OptionIssueSource}; #[turbo_tasks::value(shared)] @@ -79,4 +82,14 @@ impl Issue for AnalyzeIssue { async fn source(&self) -> Vc { Vc::cell(self.source) } + + #[turbo_tasks::function] + async fn additional_sources(&self) -> Result> { + if let Some(issue_source) = &self.source + && let Some(source) = issue_source.to_generated_code_source().await? + { + return Ok(Vc::cell(vec![source])); + } + Ok(AdditionalIssueSources::empty()) + } } diff --git a/turbopack/crates/turbopack-core/src/issue/mod.rs b/turbopack/crates/turbopack-core/src/issue/mod.rs index 8a243c82f6e8d7..773d65ffeaa130 100644 --- a/turbopack/crates/turbopack-core/src/issue/mod.rs +++ b/turbopack/crates/turbopack-core/src/issue/mod.rs @@ -28,6 +28,7 @@ use turbo_tasks_hash::{DeterministicHash, Xxh3Hash64Hasher}; use crate::{ asset::{Asset, AssetContent}, condition::ContextCondition, + generated_code_source::GeneratedCodeSource, ident::{AssetIdent, Layer}, source::Source, source_map::{GenerateSourceMap, SourceMap, TokenWithSource}, @@ -181,6 +182,15 @@ pub trait Issue { fn source(self: Vc) -> Vc { Vc::cell(None) } + + /// Additional source locations related to this issue (e.g., generated code + /// from a loader). Each source includes a description and location. + /// These are displayed alongside the primary source to give users full + /// context about the error. + #[turbo_tasks::function] + fn additional_sources(self: Vc) -> Vc { + AdditionalIssueSources::empty() + } } // A collectible trait that allows traces to be computed for a given module. @@ -620,6 +630,31 @@ impl IssueSource { pub fn file_path(&self) -> Vc { self.source.ident().path() } + + /// If this source implements `GenerateSourceMap`, returns an + /// `AdditionalIssueSource` that wraps the source in a `GeneratedCodeSource` + /// (stripping source-map support) so the generated code is shown alongside + /// the original in error messages. Returns `None` otherwise. + pub async fn to_generated_code_source(&self) -> Result> { + if ResolvedVc::try_sidecast::>(self.source).is_some() { + let description = self.source.description().await?; + let generated = Vc::upcast::>(GeneratedCodeSource::new(*self.source)) + .to_resolved() + .await?; + return Ok(Some(AdditionalIssueSource { + description: format!("Generated code of {}", description).into(), + source: IssueSource { + source: generated, + // The range is intentionally copied verbatim: the offsets + // are already in generated-source coordinates (they came + // from parsing the loader output), so no remapping is + // needed here. + range: self.range, + }, + })); + } + Ok(None) + } } impl IssueSource { @@ -701,6 +736,26 @@ pub struct OptionIssueSource(Option); #[turbo_tasks::value(transparent)] pub struct OptionStyledString(Option>); +/// A labeled issue source used to provide additional context in error messages. +/// For example, when a webpack loader produces broken code, the primary source +/// shows the original file, while an additional source shows the generated code. +#[turbo_tasks::value(shared)] +pub struct AdditionalIssueSource { + pub description: RcStr, + pub source: IssueSource, +} + +#[turbo_tasks::value(shared, transparent)] +pub struct AdditionalIssueSources(Vec); + +#[turbo_tasks::value_impl] +impl AdditionalIssueSources { + #[turbo_tasks::function] + pub fn empty() -> Vc { + Vc::cell(Vec::new()) + } +} + // A structured reference to a file with module level details for displaying in an import trace #[derive( Serialize, @@ -880,9 +935,17 @@ pub struct PlainIssue { pub documentation_link: RcStr, pub source: Option, + pub additional_sources: Vec, pub import_traces: Vec, } +#[turbo_tasks::value(serialization = "none")] +#[derive(Clone, Debug, PartialOrd, Ord)] +pub struct PlainAdditionalIssueSource { + pub description: RcStr, + pub source: PlainIssueSource, +} + fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher, full: bool) { hasher.write_ref(&issue.severity); hasher.write_ref(&issue.file_path); @@ -901,6 +964,12 @@ fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher, full: boo hasher.write_value(0_u8); } + // `additional_sources` is intentionally not hashed: it carries supplementary + // display info (e.g. generated code from a loader) that does not change the + // identity of the underlying problem. Two issues that differ only in their + // generated-code snippet still represent the same root cause and should be + // deduplicated. + if full { hasher.write_ref(&issue.import_traces); } @@ -958,6 +1027,17 @@ impl PlainIssue { None } }, + additional_sources: { + let sources = issue.additional_sources().await?; + let mut result = Vec::new(); + for s in sources.iter() { + result.push(PlainAdditionalIssueSource { + description: s.description.clone(), + source: s.source.into_plain().await?, + }); + } + result + }, import_traces: match import_tracer { Some(tracer) => { into_plain_trace( @@ -985,6 +1065,7 @@ pub struct PlainIssueSource { #[derive(Clone, Debug, PartialOrd, Ord)] pub struct PlainSource { pub ident: ReadRef, + pub file_path: ReadRef, #[turbo_tasks(debug_ignore)] pub content: ReadRef, } @@ -1001,6 +1082,7 @@ impl PlainSource { Ok(PlainSource { ident: asset.ident().to_string().await?, + file_path: asset.ident().path().to_string().await?, content, } .cell()) diff --git a/turbopack/crates/turbopack-core/src/lib.rs b/turbopack/crates/turbopack-core/src/lib.rs index bfcba4ace04ccc..61d9b5d03bdb1e 100644 --- a/turbopack/crates/turbopack-core/src/lib.rs +++ b/turbopack/crates/turbopack-core/src/lib.rs @@ -19,6 +19,7 @@ pub mod debug_id; pub mod diagnostics; pub mod environment; pub mod file_source; +pub mod generated_code_source; pub mod ident; pub mod introspect; pub mod issue; diff --git a/turbopack/crates/turbopack-core/src/source.rs b/turbopack/crates/turbopack-core/src/source.rs index 2954179b909009..d45ac4f797731e 100644 --- a/turbopack/crates/turbopack-core/src/source.rs +++ b/turbopack/crates/turbopack-core/src/source.rs @@ -1,3 +1,4 @@ +use turbo_rcstr::RcStr; use turbo_tasks::{ResolvedVc, Vc}; use crate::{asset::Asset, ident::AssetIdent}; @@ -10,6 +11,14 @@ pub trait Source: Asset { /// all properties of the [Source]. #[turbo_tasks::function] fn ident(&self) -> Vc; + + /// A human-readable description of this source, explaining where the code + /// comes from. For sources that transform another source, this should + /// include the inner source's description, creating a readable chain + /// like `"loaders [sass-loader] transform of file content of + /// ./styles.scss"`. + #[turbo_tasks::function] + fn description(&self) -> Vc; } #[turbo_tasks::value(transparent)] diff --git a/turbopack/crates/turbopack-core/src/virtual_source.rs b/turbopack/crates/turbopack-core/src/virtual_source.rs index 60adc536c980df..fbf6879ad5753d 100644 --- a/turbopack/crates/turbopack-core/src/virtual_source.rs +++ b/turbopack/crates/turbopack-core/src/virtual_source.rs @@ -1,5 +1,6 @@ use anyhow::Result; -use turbo_tasks::{ResolvedVc, Vc}; +use turbo_rcstr::RcStr; +use turbo_tasks::{ResolvedVc, ValueToString, Vc}; use turbo_tasks_fs::FileSystemPath; use crate::{ @@ -40,6 +41,12 @@ impl Source for VirtualSource { fn ident(&self) -> Vc { *self.ident } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let ident = self.ident.to_string().await?; + Ok(Vc::cell(format!("virtual source {}", ident).into())) + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-css/src/process.rs b/turbopack/crates/turbopack-css/src/process.rs index 8d604880c66b98..c46321f9ca1b08 100644 --- a/turbopack/crates/turbopack-css/src/process.rs +++ b/turbopack/crates/turbopack-css/src/process.rs @@ -23,8 +23,8 @@ use turbopack_core::{ chunk::{ChunkingContext, MinifyType}, environment::Environment, issue::{ - Issue, IssueExt, IssueSource, IssueStage, OptionIssueSource, OptionStyledString, - StyledString, + AdditionalIssueSources, Issue, IssueExt, IssueSource, IssueStage, OptionIssueSource, + OptionStyledString, StyledString, }, reference::ModuleReferences, reference_type::ImportContext, @@ -746,6 +746,14 @@ impl Issue for ParsingIssue { StyledString::Text(self.msg.clone()).resolved_cell(), ))) } + + #[turbo_tasks::function] + async fn additional_sources(&self) -> Result> { + if let Some(source) = self.source.to_generated_code_source().await? { + return Ok(Vc::cell(vec![source])); + } + Ok(AdditionalIssueSources::empty()) + } } #[cfg(test)] diff --git a/turbopack/crates/turbopack-ecmascript/src/text/mod.rs b/turbopack/crates/turbopack-ecmascript/src/text/mod.rs index 6f66866b3993f4..36f6ea1cbb176d 100644 --- a/turbopack/crates/turbopack-ecmascript/src/text/mod.rs +++ b/turbopack/crates/turbopack-ecmascript/src/text/mod.rs @@ -34,6 +34,12 @@ impl Source for TextContentFileSource { .with_modifier(rcstr!("text content")) .rename_as(rcstr!("*.mjs")) } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let inner = self.source.description().await?; + Ok(Vc::cell(format!("text content of {}", inner).into())) + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-env/src/asset.rs b/turbopack/crates/turbopack-env/src/asset.rs index 4f304a1ef069cd..0689690526e630 100644 --- a/turbopack/crates/turbopack-env/src/asset.rs +++ b/turbopack/crates/turbopack-env/src/asset.rs @@ -1,6 +1,7 @@ use std::io::Write; use anyhow::Result; +use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; use turbo_tasks_env::ProcessEnv; use turbo_tasks_fs::{File, FileContent, FileSystemPath, rope::RopeBuilder}; @@ -36,6 +37,11 @@ impl Source for ProcessEnvAsset { fn ident(&self) -> Result> { Ok(AssetIdent::from_path(self.root.join(".env.js")?)) } + + #[turbo_tasks::function] + fn description(&self) -> Vc { + Vc::cell(rcstr!("process environment")) + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-mdx/src/lib.rs b/turbopack/crates/turbopack-mdx/src/lib.rs index 2e9a95d105adef..3fc2da2b2161fb 100644 --- a/turbopack/crates/turbopack-mdx/src/lib.rs +++ b/turbopack/crates/turbopack-mdx/src/lib.rs @@ -113,6 +113,12 @@ impl Source for MdxTransformedAsset { fn ident(&self) -> Vc { self.source.ident().rename_as(rcstr!("*.tsx")) } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let inner = self.source.description().await?; + Ok(Vc::cell(format!("MDX transform of {}", inner).into())) + } } #[turbo_tasks::value_impl] diff --git a/turbopack/crates/turbopack-node/src/transforms/postcss.rs b/turbopack/crates/turbopack-node/src/transforms/postcss.rs index d198d5d48b977b..5b441db366294b 100644 --- a/turbopack/crates/turbopack-node/src/transforms/postcss.rs +++ b/turbopack/crates/turbopack-node/src/transforms/postcss.rs @@ -175,6 +175,12 @@ impl Source for PostCssTransformedAsset { fn ident(&self) -> Vc { self.source.ident() } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let inner = self.source.description().await?; + Ok(Vc::cell(format!("PostCSS transform of {}", inner).into())) + } } #[turbo_tasks::value_impl] @@ -284,6 +290,11 @@ impl JsonSource { #[turbo_tasks::value_impl] impl Source for JsonSource { + #[turbo_tasks::function] + fn description(&self) -> Vc { + Vc::cell(format!("JSON content of {}", self.path).into()) + } + #[turbo_tasks::function] async fn ident(&self) -> Result> { match &*self.key.await? { diff --git a/turbopack/crates/turbopack-node/src/transforms/webpack.rs b/turbopack/crates/turbopack-node/src/transforms/webpack.rs index dd89d6782df0d6..dd5b8bc32ae4b7 100644 --- a/turbopack/crates/turbopack-node/src/transforms/webpack.rs +++ b/turbopack/crates/turbopack-node/src/transforms/webpack.rs @@ -11,8 +11,8 @@ use serde_with::serde_as; use tracing::Instrument; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ - Completion, OperationVc, ReadRef, ResolvedVc, TaskInput, TryJoinIterExt, Vc, - trace::TraceRawVcs, turbobail, + Completion, OperationVc, ReadRef, ResolvedVc, TaskInput, TryJoinIterExt, ValueToString, Vc, + trace::TraceRawVcs, }; use turbo_tasks_env::ProcessEnv; use turbo_tasks_fs::{ @@ -154,6 +154,21 @@ impl Source for WebpackLoadersProcessedAsset { }, ) } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let inner = self.source.description().await?; + let loaders = self.transform.await?.loaders.await?; + let loader_names: Vec<&str> = loaders.iter().map(|l| l.loader.as_str()).collect(); + Ok(Vc::cell( + format!( + "loaders [{}] transform of {}", + loader_names.join(", "), + inner + ) + .into(), + )) + } } #[turbo_tasks::value_impl] @@ -262,9 +277,10 @@ impl WebpackLoadersProcessedAsset { let resource_fs_path = self.source.ident().path().await?; let Some(resource_path) = project_path.get_relative_path_to(&resource_fs_path) else { - turbobail!( - "Resource path \"{resource_fs_path}\" needs to be on project filesystem \ - \"{project_path}\"", + bail!( + "Resource path \"{}\" needs to be on project filesystem \"{}\"", + resource_fs_path, + project_path ); }; let config_value = evaluate_webpack_loader(WebpackLoaderContext { @@ -610,13 +626,18 @@ impl EvaluateContext for WebpackLoaderContext { { Ok(ResponseMessage::Resolve { path }) } else { - turbobail!( - "Resolving {request} in {lookup_path} ends up on a different \ - filesystem" + bail!( + "Resolving {} in {} ends up on a different filesystem", + request.to_string().await?, + lookup_path.value_to_string().await? ); } } else { - turbobail!("Unable to resolve {request} in {lookup_path}"); + bail!( + "Unable to resolve {} in {}", + request.to_string().await?, + lookup_path.value_to_string().await? + ); } } RequestMessage::TrackFileRead { file } => { diff --git a/turbopack/crates/turbopack-wasm/src/source.rs b/turbopack/crates/turbopack-wasm/src/source.rs index 403bae2a5323f6..e9609ff91a6e49 100644 --- a/turbopack/crates/turbopack-wasm/src/source.rs +++ b/turbopack/crates/turbopack-wasm/src/source.rs @@ -1,5 +1,6 @@ use anyhow::Result; use bincode::{Decode, Encode}; +use turbo_rcstr::RcStr; use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, Vc, trace::TraceRawVcs}; use turbo_tasks_fs::{File, FileContent}; use turbopack_core::{ @@ -59,6 +60,14 @@ impl Source for WebAssemblySource { .with_path(self.source.ident().path().await?.append("_.wasm")?), }) } + + #[turbo_tasks::function] + async fn description(&self) -> Result> { + let inner = self.source.description().await?; + Ok(Vc::cell( + format!("WebAssembly transform of {}", inner).into(), + )) + } } #[turbo_tasks::value_impl] From 46789444d5b5089477d8704dba759ef5a1807242 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:10:22 +0100 Subject: [PATCH 6/6] Remove unused layer from server actions manifest (#91425) ``` // This is only used by Webpack to correctly output the manifest. It's value shouldn't be relied // upon externally. It's possible that the same action can be in different layers in a single // page, which cannot be modelled with this API anyway. ``` --- crates/next-api/src/server_actions.rs | 3 +- crates/next-core/src/next_manifests/mod.rs | 2 - .../plugins/flight-client-entry-plugin.ts | 22 +++---- .../shared/lib/turbopack/manifest-loader.ts | 4 -- .../actions-tree-shaking/_testing/utils.ts | 22 ++----- .../actions-tree-shaking/basic/basic.test.ts | 28 ++++----- .../mixed-module-actions.test.ts | 24 +++---- .../reexport/reexport.test.ts | 62 ++++++++----------- .../shared-module-actions.test.ts | 40 +++++------- .../use-effect-actions.test.ts | 12 ++-- 10 files changed, 84 insertions(+), 135 deletions(-) diff --git a/crates/next-api/src/server_actions.rs b/crates/next-api/src/server_actions.rs index 3b16af47796edf..33a6d1e39274f0 100644 --- a/crates/next-api/src/server_actions.rs +++ b/crates/next-api/src/server_actions.rs @@ -198,7 +198,7 @@ async fn build_manifest( } // Now create the manifest entries - for (hash_id, (layer, name, filename)) in &action_metadata { + for (hash_id, (_layer, name, filename)) in &action_metadata { let entry = mapping.entry(hash_id.as_str()).or_default(); entry.workers.insert( &key, @@ -211,7 +211,6 @@ async fn build_manifest( filename: filename.as_str(), }, ); - entry.layer.insert(&key, *layer); // Hoist the filename and exported_name to the entry level entry.exported_name = name.as_str(); diff --git a/crates/next-core/src/next_manifests/mod.rs b/crates/next-core/src/next_manifests/mod.rs index 58c57eb5193cd1..3d375b212a86a2 100644 --- a/crates/next-core/src/next_manifests/mod.rs +++ b/crates/next-core/src/next_manifests/mod.rs @@ -385,8 +385,6 @@ pub struct ActionManifestEntry<'a> { /// module that exports it. pub workers: FxIndexMap<&'a str, ActionManifestWorkerEntry<'a>>, - pub layer: FxIndexMap<&'a str, ActionLayer>, - #[serde(rename = "exportedName")] pub exported_name: &'a str, diff --git a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts index 45757716b517f3..508d1b5a735af8 100644 --- a/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts @@ -74,8 +74,12 @@ type Actions = { async: boolean } } - // Record which layer the action is in (rsc or sc_action), in the specific entry. - layer: { + // Record which layer the action is in (rsc or sc_action), in the specific entry + // + // This is only used by Webpack to correctly output the manifest. It's value shouldn't be relied + // upon externally. It's possible that the same action can be in different layers in a single + // page, which cannot be modelled with this API anyway. + layer?: { [name: string]: string } } @@ -1016,7 +1020,7 @@ export class FlightClientEntryPlugin { async: false, } - currentCompilerServerActions[id].layer[bundlePath] = fromClient + currentCompilerServerActions[id].layer![bundlePath] = fromClient ? WEBPACK_LAYERS.actionBrowser : WEBPACK_LAYERS.reactServerComponents } @@ -1124,13 +1128,11 @@ export class FlightClientEntryPlugin { }) for (let id in pluginState.serverActions) { - const action = pluginState.serverActions[id] + const { layer, ...action } = pluginState.serverActions[id] for (let name in action.workers) { const modId = pluginState.serverActionModules[name][ - action.layer[name] === WEBPACK_LAYERS.actionBrowser - ? 'client' - : 'server' + layer![name] === WEBPACK_LAYERS.actionBrowser ? 'client' : 'server' ] action.workers[name] = modId! } @@ -1138,13 +1140,11 @@ export class FlightClientEntryPlugin { } for (let id in pluginState.edgeServerActions) { - const action = pluginState.edgeServerActions[id] + const { layer, ...action } = pluginState.edgeServerActions[id] for (let name in action.workers) { const modId = pluginState.edgeServerActionModules[name][ - action.layer[name] === WEBPACK_LAYERS.actionBrowser - ? 'client' - : 'server' + layer![name] === WEBPACK_LAYERS.actionBrowser ? 'client' : 'server' ] action.workers[name] = modId! } diff --git a/packages/next/src/shared/lib/turbopack/manifest-loader.ts b/packages/next/src/shared/lib/turbopack/manifest-loader.ts index 9caf9547c70b1b..9b2c09bf44332f 100644 --- a/packages/next/src/shared/lib/turbopack/manifest-loader.ts +++ b/packages/next/src/shared/lib/turbopack/manifest-loader.ts @@ -273,12 +273,10 @@ export class TurbopackManifestLoader { for (const key in other) { const action = (actionEntries[key] ??= { workers: {}, - layer: {}, }) action.filename = other[key].filename action.exportedName = other[key].exportedName Object.assign(action.workers, other[key].workers) - Object.assign(action.layer, other[key].layer) } } @@ -289,12 +287,10 @@ export class TurbopackManifestLoader { for (const key in manifest.node) { const entry = manifest.node[key] entry.workers = sortObjectByKey(entry.workers) - entry.layer = sortObjectByKey(entry.layer) } for (const key in manifest.edge) { const entry = manifest.edge[key] entry.workers = sortObjectByKey(entry.workers) - entry.layer = sortObjectByKey(entry.layer) } return manifest diff --git a/test/production/app-dir/actions-tree-shaking/_testing/utils.ts b/test/production/app-dir/actions-tree-shaking/_testing/utils.ts index fd49219fcd2b4c..fbb46c75f4b61c 100644 --- a/test/production/app-dir/actions-tree-shaking/_testing/utils.ts +++ b/test/production/app-dir/actions-tree-shaking/_testing/utils.ts @@ -12,9 +12,6 @@ type Actions = { async: boolean } } - layer: { - [name: string]: string - } } } @@ -50,9 +47,7 @@ export function nextTestSetupActionTreeShaking(opts) { } type ActionState = { - [route: string]: { - [layer: string]: string[] - } + [route: string]: string[] } function getActionsRoutesState(actionsMappingOfRuntime: Actions): ActionState { @@ -61,12 +56,7 @@ function getActionsRoutesState(actionsMappingOfRuntime: Actions): ActionState { const action = actionsMappingOfRuntime[actionId] for (const routePath in action.workers) { if (!state[routePath]) { - state[routePath] = {} - } - const layer = action.layer[routePath] - - if (!state[routePath][layer]) { - state[routePath][layer] = [] + state[routePath] = [] } // Normalize when NEXT_SKIP_ISOLATE=1 @@ -75,14 +65,12 @@ function getActionsRoutesState(actionsMappingOfRuntime: Actions): ActionState { action.filename.indexOf('/', 'test/tmp/next-test-'.length) + 1 ) : action.filename - state[routePath][layer].push(`${filename}#${action.exportedName}`) + state[routePath].push(`${filename}#${action.exportedName}`) } } - for (const layer of Object.values(state)) { - for (const list of Object.values(layer)) { - list.sort() - } + for (const page of Object.values(state)) { + page.sort() } return state diff --git a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts index ea7303a2023a7c..a09aaf81bc56f2 100644 --- a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts +++ b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts @@ -15,23 +15,17 @@ import { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) expect(actionsRoutesState).toMatchInlineSnapshot(` { - "app/client/page": { - "action-browser": [ - "app/actions.js#clientComponentAction", - ], - }, - "app/inline/page": { - "rsc": [ - "app/inline/page.js#$$RSC_SERVER_ACTION_0", - ], - }, - "app/server/page": { - "rsc": [ - "app/actions.js#clientComponentAction", - "app/actions.js#serverComponentAction", - "app/actions.js#unusedExportedAction", - ], - }, + "app/client/page": [ + "app/actions.js#clientComponentAction", + ], + "app/inline/page": [ + "app/inline/page.js#$$RSC_SERVER_ACTION_0", + ], + "app/server/page": [ + "app/actions.js#clientComponentAction", + "app/actions.js#serverComponentAction", + "app/actions.js#unusedExportedAction", + ], } `) }) diff --git a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts index ce333cf8c46e56..16337c25d048b0 100644 --- a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts @@ -15,20 +15,16 @@ import { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) expect(actionsRoutesState).toMatchInlineSnapshot(` { - "app/mixed-module/cjs/page": { - "rsc": [ - "app/mixed-module/cjs/actions.js#cjsModuleTypeAction", - "app/mixed-module/cjs/actions.js#esmModuleTypeAction", - "app/mixed-module/cjs/actions.js#unusedModuleTypeAction1", - ], - }, - "app/mixed-module/esm/page": { - "rsc": [ - "app/mixed-module/esm/actions.js#cjsModuleTypeAction", - "app/mixed-module/esm/actions.js#esmModuleTypeAction", - "app/mixed-module/esm/actions.js#unusedModuleTypeAction1", - ], - }, + "app/mixed-module/cjs/page": [ + "app/mixed-module/cjs/actions.js#cjsModuleTypeAction", + "app/mixed-module/cjs/actions.js#esmModuleTypeAction", + "app/mixed-module/cjs/actions.js#unusedModuleTypeAction1", + ], + "app/mixed-module/esm/page": [ + "app/mixed-module/esm/actions.js#cjsModuleTypeAction", + "app/mixed-module/esm/actions.js#esmModuleTypeAction", + "app/mixed-module/esm/actions.js#unusedModuleTypeAction1", + ], } `) }) diff --git a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts index 707511b1c5ac52..fd71a4f8c26cf5 100644 --- a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts +++ b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts @@ -18,43 +18,31 @@ import { retry } from 'next-test-utils' expect(actionsRoutesState).toMatchInlineSnapshot(` { - "app/named-reexport/client/page": { - "action-browser": [ - "app/named-reexport/client/actions.js#sharedClientLayerAction", - ], - }, - "app/named-reexport/server/page": { - "rsc": [ - "app/named-reexport/server/actions.js#sharedServerLayerAction", - "app/named-reexport/server/actions.js#unusedServerLayerAction1", - "app/named-reexport/server/actions.js#unusedServerLayerAction2", - ], - }, - "app/namespace-reexport-2/client/page": { - "action-browser": [ - "app/namespace-reexport-2/actions/action-modules.js#action", - "app/namespace-reexport-2/nested.js#getFoo", - ], - }, - "app/namespace-reexport-2/server/page": { - "rsc": [ - "app/namespace-reexport-2/actions/action-modules.js#action", - "app/namespace-reexport-2/nested.js#foo", - "app/namespace-reexport-2/nested.js#getFoo", - ], - }, - "app/namespace-reexport/client/page": { - "action-browser": [ - "app/namespace-reexport/client/actions.js#sharedClientLayerAction", - ], - }, - "app/namespace-reexport/server/page": { - "rsc": [ - "app/namespace-reexport/server/actions.js#sharedServerLayerAction", - "app/namespace-reexport/server/actions.js#unusedServerLayerAction1", - "app/namespace-reexport/server/actions.js#unusedServerLayerAction2", - ], - }, + "app/named-reexport/client/page": [ + "app/named-reexport/client/actions.js#sharedClientLayerAction", + ], + "app/named-reexport/server/page": [ + "app/named-reexport/server/actions.js#sharedServerLayerAction", + "app/named-reexport/server/actions.js#unusedServerLayerAction1", + "app/named-reexport/server/actions.js#unusedServerLayerAction2", + ], + "app/namespace-reexport-2/client/page": [ + "app/namespace-reexport-2/actions/action-modules.js#action", + "app/namespace-reexport-2/nested.js#getFoo", + ], + "app/namespace-reexport-2/server/page": [ + "app/namespace-reexport-2/actions/action-modules.js#action", + "app/namespace-reexport-2/nested.js#foo", + "app/namespace-reexport-2/nested.js#getFoo", + ], + "app/namespace-reexport/client/page": [ + "app/namespace-reexport/client/actions.js#sharedClientLayerAction", + ], + "app/namespace-reexport/server/page": [ + "app/namespace-reexport/server/actions.js#sharedServerLayerAction", + "app/namespace-reexport/server/actions.js#unusedServerLayerAction1", + "app/namespace-reexport/server/actions.js#unusedServerLayerAction2", + ], } `) }) diff --git a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts index 21ba4dc9e086a2..0cb42ac1997b89 100644 --- a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts @@ -15,30 +15,22 @@ import { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) expect(actionsRoutesState).toMatchInlineSnapshot(` { - "app/client/one/page": { - "action-browser": [ - "app/client/actions.js#sharedClientLayerAction", - ], - }, - "app/client/two/page": { - "action-browser": [ - "app/client/actions.js#sharedClientLayerAction", - ], - }, - "app/server/one/page": { - "rsc": [ - "app/server/actions.js#sharedServerLayerAction", - "app/server/actions.js#unusedServerLayerAction1", - "app/server/actions.js#unusedServerLayerAction2", - ], - }, - "app/server/two/page": { - "rsc": [ - "app/server/actions.js#sharedServerLayerAction", - "app/server/actions.js#unusedServerLayerAction1", - "app/server/actions.js#unusedServerLayerAction2", - ], - }, + "app/client/one/page": [ + "app/client/actions.js#sharedClientLayerAction", + ], + "app/client/two/page": [ + "app/client/actions.js#sharedClientLayerAction", + ], + "app/server/one/page": [ + "app/server/actions.js#sharedServerLayerAction", + "app/server/actions.js#unusedServerLayerAction1", + "app/server/actions.js#unusedServerLayerAction2", + ], + "app/server/two/page": [ + "app/server/actions.js#sharedServerLayerAction", + "app/server/actions.js#unusedServerLayerAction1", + "app/server/actions.js#unusedServerLayerAction2", + ], } `) }) diff --git a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts index ef87213ce185d4..d3ba7146345d72 100644 --- a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts @@ -12,13 +12,11 @@ describe('actions-tree-shaking - use-effect-actions', () => { const actionsRoutesState = await getActionsRoutesStateByRuntime(next) expect(actionsRoutesState).toMatchInlineSnapshot(` { - "app/mixed/page": { - "action-browser": [ - "app/mixed/actions.ts#action1", - "app/mixed/actions.ts#action2", - "app/mixed/actions.ts#action3", - ], - }, + "app/mixed/page": [ + "app/mixed/actions.ts#action1", + "app/mixed/actions.ts#action2", + "app/mixed/actions.ts#action3", + ], } `) })