From 01117b24dc1b0d345cb67d62b5f9877d7505aadf Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Tue, 15 Oct 2024 22:05:34 +0200 Subject: [PATCH 1/3] Add Turbopack support for `'use cache'` in route handlers (#71306) Adds the following missing pieces to Turbopack: - apply server action transforms to route handler modules - generate client reference manifests for route handler app endpoints - make `experimental.cacheLife` config serialisable, to be injected into the edge route handler modules - the `nextConfig` injection was already implemented in: #71258 --- crates/next-api/src/app.rs | 28 ++++--- crates/next-core/src/next_config.rs | 56 +++++++++++++ .../next-core/src/next_server/transforms.rs | 7 ++ .../dynamic-io/dynamic-io.routes.test.ts | 82 +++++++++---------- .../use-cache-route-handler-only.test.ts | 9 +- test/e2e/app-dir/use-cache/use-cache.test.ts | 55 ++++++------- 6 files changed, 143 insertions(+), 94 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index d29fdc2c9f08c..36a5ff0c378b4 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -815,13 +815,12 @@ impl AppEndpoint { let app_entry = self.app_endpoint_entry().await?; - let (process_client, process_ssr) = match this.ty { - AppEndpointType::Page { ty, .. } => (true, matches!(ty, AppPageEndpointType::Html)), - // NOTE(alexkirsz) For routes, technically, a lot of the following code is not needed, - // as we know we won't have any client references. However, for now, for simplicity's - // sake, we just do the same thing as for pages. - AppEndpointType::Route { .. } => (false, false), - AppEndpointType::Metadata { .. } => (false, false), + let (process_client_components, process_client_assets, process_ssr) = match this.ty { + AppEndpointType::Page { ty, .. } => { + (true, true, matches!(ty, AppPageEndpointType::Html)) + } + AppEndpointType::Route { .. } => (true, false, false), + AppEndpointType::Metadata { .. } => (false, false, false), }; let node_root = this.app_project.project().node_root(); @@ -843,7 +842,7 @@ impl AppEndpoint { let client_chunking_context = this.app_project.project().client_chunking_context(); let (app_server_reference_modules, client_dynamic_imports, client_references) = - if process_client { + if process_client_components { let client_shared_chunk_group = get_app_client_shared_chunk_group( AssetIdent::from_path(this.app_project.project().project_path()) .with_modifier(client_shared_chunks()), @@ -902,7 +901,7 @@ impl AppEndpoint { NextRuntime::Edge => this .app_project .project() - .edge_chunking_context(process_client), + .edge_chunking_context(process_client_assets), }) } else { None @@ -1138,7 +1137,7 @@ impl AppEndpoint { let chunking_context = this .app_project .project() - .edge_chunking_context(process_client); + .edge_chunking_context(process_client_assets); let mut evaluatable_assets = this .app_project .edge_rsc_runtime_entries() @@ -1300,7 +1299,7 @@ impl AppEndpoint { let chunking_context = this .app_project .project() - .server_chunking_context(process_client); + .server_chunking_context(process_client_assets); if let Some(app_server_reference_modules) = app_server_reference_modules { let (loader, manifest) = create_server_actions_manifest( @@ -1354,7 +1353,12 @@ impl AppEndpoint { .server_component_entries .iter() .copied() - .take(client_references.server_component_entries.len() - 1) + .take( + client_references + .server_component_entries + .len() + .saturating_sub(1), + ) { let span = tracing::trace_span!( "layout segment", diff --git a/crates/next-core/src/next_config.rs b/crates/next-core/src/next_config.rs index 7c6e5664e5215..fc9c5723ca9e4 100644 --- a/crates/next-core/src/next_config.rs +++ b/crates/next-core/src/next_config.rs @@ -527,6 +527,7 @@ pub struct ExperimentalConfig { after: Option, amp: Option, app_document_preloading: Option, + cache_life: Option>, case_sensitive_routes: Option, cpus: Option, cra_compat: Option, @@ -581,6 +582,61 @@ pub struct ExperimentalConfig { worker_threads: Option, } +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, TraceRawVcs)] +#[serde(rename_all = "camelCase")] +pub struct CacheLifeProfile { + #[serde(skip_serializing_if = "Option::is_none")] + pub stale: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub revalidate: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub expire: Option, +} + +#[test] +fn test_cache_life_profiles() { + let json = serde_json::json!({ + "cacheLife": { + "frequent": { + "stale": 19, + "revalidate": 100, + }, + } + }); + + let config: ExperimentalConfig = serde_json::from_value(json).unwrap(); + let mut expected_cache_life = FxIndexMap::default(); + + expected_cache_life.insert( + "frequent".to_string(), + CacheLifeProfile { + stale: Some(19), + revalidate: Some(100), + expire: None, + }, + ); + + assert_eq!(config.cache_life, Some(expected_cache_life)); +} + +#[test] +fn test_cache_life_profiles_invalid() { + let json = serde_json::json!({ + "cacheLife": { + "invalid": { + "stale": "invalid_value", + }, + } + }); + + let result: Result = serde_json::from_value(json); + + assert!( + result.is_err(), + "Deserialization should fail due to invalid 'stale' value type" + ); +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, TraceRawVcs)] #[serde(rename_all = "lowercase")] pub enum ExperimentalPartialPrerenderingIncrementalValue { diff --git a/crates/next-core/src/next_server/transforms.rs b/crates/next-core/src/next_server/transforms.rs index c65a52e6880a2..9f92f50776725 100644 --- a/crates/next-core/src/next_server/transforms.rs +++ b/crates/next-core/src/next_server/transforms.rs @@ -90,6 +90,7 @@ pub async fn get_next_server_transforms_rules( ActionsTransform::Client, mdx_rs, )); + is_app_dir = true; false @@ -105,7 +106,13 @@ pub async fn get_next_server_transforms_rules( true } ServerContextType::AppRoute { .. } => { + rules.push(get_server_actions_transform_rule( + ActionsTransform::Server, + mdx_rs, + )); + is_app_dir = true; + false } ServerContextType::Middleware { .. } | ServerContextType::Instrumentation { .. } => false, diff --git a/test/e2e/app-dir/dynamic-io/dynamic-io.routes.test.ts b/test/e2e/app-dir/dynamic-io/dynamic-io.routes.test.ts index 68fa1d10df3ec..59244110a4767 100644 --- a/test/e2e/app-dir/dynamic-io/dynamic-io.routes.test.ts +++ b/test/e2e/app-dir/dynamic-io/dynamic-io.routes.test.ts @@ -1,8 +1,7 @@ -/* eslint-disable jest/no-standalone-expect */ import { nextTestSetup } from 'e2e-utils' describe('dynamic-io', () => { - const { next, isNextDev, isTurbopack, skipped } = nextTestSetup({ + const { next, isNextDev, skipped } = nextTestSetup({ files: __dirname, skipDeployment: true, }) @@ -11,8 +10,6 @@ describe('dynamic-io', () => { return } - const itSkipTurbopack = isTurbopack ? it.skip : it - let cliIndex = 0 beforeEach(() => { cliIndex = next.cliOutput.length @@ -209,56 +206,53 @@ describe('dynamic-io', () => { expect(message2).toEqual(json.message2) }) - itSkipTurbopack( - 'should prerender GET route handlers that have entirely cached io ("use cache")', - async () => { - let str = await next.render('/routes/use_cache-cached', {}) - let json = JSON.parse(str) - - let message1 = json.message1 - let message2 = json.message2 - - if (isNextDev) { - expect(json.value).toEqual('at runtime') - expect(typeof message1).toBe('string') - expect(typeof message2).toBe('string') - } else { - expect(json.value).toEqual('at buildtime') - expect(typeof message1).toBe('string') - expect(typeof message2).toBe('string') - } - - str = await next.render('/routes/use_cache-cached', {}) - json = JSON.parse(str) - - if (isNextDev) { - expect(json.value).toEqual('at runtime') - expect(message1).toEqual(json.message1) - expect(message2).toEqual(json.message2) - } else { - expect(json.value).toEqual('at buildtime') - expect(message1).toEqual(json.message1) - expect(message2).toEqual(json.message2) - } - - str = await next.render('/routes/-edge/use_cache-cached', {}) - json = JSON.parse(str) - - message1 = json.message1 - message2 = json.message2 + it('should prerender GET route handlers that have entirely cached io ("use cache")', async () => { + let str = await next.render('/routes/use_cache-cached', {}) + let json = JSON.parse(str) + + let message1 = json.message1 + let message2 = json.message2 + if (isNextDev) { expect(json.value).toEqual('at runtime') expect(typeof message1).toBe('string') expect(typeof message2).toBe('string') + } else { + expect(json.value).toEqual('at buildtime') + expect(typeof message1).toBe('string') + expect(typeof message2).toBe('string') + } - str = await next.render('/routes/-edge/use_cache-cached', {}) - json = JSON.parse(str) + str = await next.render('/routes/use_cache-cached', {}) + json = JSON.parse(str) + if (isNextDev) { expect(json.value).toEqual('at runtime') expect(message1).toEqual(json.message1) expect(message2).toEqual(json.message2) + } else { + expect(json.value).toEqual('at buildtime') + expect(message1).toEqual(json.message1) + expect(message2).toEqual(json.message2) } - ) + + str = await next.render('/routes/-edge/use_cache-cached', {}) + json = JSON.parse(str) + + message1 = json.message1 + message2 = json.message2 + + expect(json.value).toEqual('at runtime') + expect(typeof message1).toBe('string') + expect(typeof message2).toBe('string') + + str = await next.render('/routes/-edge/use_cache-cached', {}) + json = JSON.parse(str) + + expect(json.value).toEqual('at runtime') + expect(message1).toEqual(json.message1) + expect(message2).toEqual(json.message2) + }) it('should not prerender GET route handlers that have some uncached io (unstable_cache)', async () => { let str = await next.render('/routes/io-mixed', {}) diff --git a/test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts b/test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts index 37a9d3086ebb3..a31a3e79b0db5 100644 --- a/test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts +++ b/test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts @@ -1,23 +1,20 @@ -/* eslint-disable jest/no-standalone-expect */ import { nextTestSetup } from 'e2e-utils' // Explicitly don't mix route handlers with pages in this test app, to make sure // that this also works in isolation. describe('use-cache-route-handler-only', () => { - const { next, isTurbopack } = nextTestSetup({ + const { next } = nextTestSetup({ files: __dirname, }) - const itSkipTurbopack = isTurbopack ? it.skip : it - - itSkipTurbopack('should cache results in node route handlers', async () => { + it('should cache results in node route handlers', async () => { const response = await next.fetch('/node') const { rand1, rand2 } = await response.json() expect(rand1).toEqual(rand2) }) - itSkipTurbopack('should cache results in edge route handlers', async () => { + it('should cache results in edge route handlers', async () => { const response = await next.fetch('/edge') const { rand1, rand2 } = await response.json() diff --git a/test/e2e/app-dir/use-cache/use-cache.test.ts b/test/e2e/app-dir/use-cache/use-cache.test.ts index a7391bd90b806..28b50089745ff 100644 --- a/test/e2e/app-dir/use-cache/use-cache.test.ts +++ b/test/e2e/app-dir/use-cache/use-cache.test.ts @@ -76,7 +76,7 @@ describe('use-cache', () => { } }) - itSkipTurbopack('should cache results in route handlers', async () => { + it('should cache results in route handlers', async () => { const response = await next.fetch('/api') const { rand1, rand2 } = await response.json() @@ -84,38 +84,29 @@ describe('use-cache', () => { }) if (isNextStart) { - itSkipTurbopack( - 'should match the expected revalidate config on the prerender manifest', - async () => { - const prerenderManifest = JSON.parse( - await next.readFile('.next/prerender-manifest.json') - ) - - expect(prerenderManifest.version).toBe(4) - expect( - prerenderManifest.routes['/cache-life'].initialRevalidateSeconds - ).toBe(100) - } - ) + it('should match the expected revalidate config on the prerender manifest', async () => { + const prerenderManifest = JSON.parse( + await next.readFile('.next/prerender-manifest.json') + ) - itSkipTurbopack( - 'should match the expected stale config in the page header', - async () => { - const meta = JSON.parse( - await next.readFile('.next/server/app/cache-life.meta') - ) - expect(meta.headers['x-nextjs-stale-time']).toBe('19') - } - ) + expect(prerenderManifest.version).toBe(4) + expect( + prerenderManifest.routes['/cache-life'].initialRevalidateSeconds + ).toBe(100) + }) - itSkipTurbopack( - 'should propagate unstable_cache tags correctly', - async () => { - const meta = JSON.parse( - await next.readFile('.next/server/app/cache-tag.meta') - ) - expect(meta.headers['x-next-cache-tags']).toContain('a,c,b') - } - ) + it('should match the expected stale config in the page header', async () => { + const meta = JSON.parse( + await next.readFile('.next/server/app/cache-life.meta') + ) + expect(meta.headers['x-nextjs-stale-time']).toBe('19') + }) + + it('should propagate unstable_cache tags correctly', async () => { + const meta = JSON.parse( + await next.readFile('.next/server/app/cache-tag.meta') + ) + expect(meta.headers['x-next-cache-tags']).toContain('a,c,b') + }) } }) From fc6958fe5028e634335d030c8763a7bb1cffc57c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 15 Oct 2024 16:09:38 -0400 Subject: [PATCH 2/3] Add built-in set of cacheLife profiles (#71322) ``` cacheLife: { "seconds": { stale: 30, // 30 seconds revalidate: 1, // 1 second expire: 1, // 1 minute }, "minutes": { stale: 60 * 5, // 5 minutes revalidate: 60, // 1 minute expire: 60 * 60, // 1 hour }, "hours": { stale: 60 * 5, // 5 minutes revalidate: 60 * 60, // 1 hour expire: 60 * 60 * 24, // 1 day }, "days": { stale: 60 * 5, // 5 minutes revalidate: 60 * 60 * 24, // 1 day expire: 60 * 60 * 24 * 7, // 1 week }, "weeks": { stale: 60 * 5, // 5 minutes revalidate: 60 * 60 * 24 * 7, // 1 week expire: 60 * 60 * 24 * 30, // 1 month }, "max": { stale: 60 * 5, // 5 minutes revalidate: 60 * 60 * 24 * 30, // 1 month expire: Infinity, // Unbounded. Whatever max your host stores. } } ``` The stale time is either 30 seconds or 5 minutes which correspond to our existing default for dynamic/static data. If you have overridden the `staleTimes` options, we default for those. `"seconds"` means basically dynamic. The revalidate time is the beginning of the range and the expire is the end of that range. So `"days"` refreshes somewhere after 1 day and before 1 week. --- packages/next/src/server/config-shared.ts | 32 ++++++++++++++++++- packages/next/src/server/config.ts | 12 +++++++ .../next/src/server/use-cache/cache-life.ts | 11 ++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/config-shared.ts b/packages/next/src/server/config-shared.ts index 5a92d7e37377c..2e4987e19c805 100644 --- a/packages/next/src/server/config-shared.ts +++ b/packages/next/src/server/config-shared.ts @@ -1031,9 +1031,39 @@ export const defaultConfig: NextConfig = { cacheLife: { default: { stale: undefined, // defaults to staleTimes.static - revalidate: 900, + revalidate: 60 * 15, // 15 minutes expire: INFINITE_CACHE, }, + seconds: { + stale: undefined, // defaults to staleTimes.dynamic + revalidate: 1, // 1 second + expire: 1, // 1 minute + }, + minutes: { + stale: 60 * 5, // 5 minutes + revalidate: 60, // 1 minute + expire: 60 * 60, // 1 hour + }, + hours: { + stale: 60 * 5, // 5 minutes + revalidate: 60 * 60, // 1 hour + expire: 60 * 60 * 24, // 1 day + }, + days: { + stale: 60 * 5, // 5 minutes + revalidate: 60 * 60 * 24, // 1 day + expire: 60 * 60 * 24 * 7, // 1 week + }, + weeks: { + stale: 60 * 5, // 5 minutes + revalidate: 60 * 60 * 24 * 7, // 1 week + expire: 60 * 60 * 24 * 30, // 1 month + }, + max: { + stale: 60 * 5, // 5 minutes + revalidate: 60 * 60 * 24 * 30, // 1 month + expire: INFINITE_CACHE, // Unbounded. + }, }, multiZoneDraftMode: false, appNavFailHandling: Boolean(process.env.NEXT_PRIVATE_FLYING_SHUTTLE), diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts index 0ed3fef91edf7..4fbaffd4c6f92 100644 --- a/packages/next/src/server/config.ts +++ b/packages/next/src/server/config.ts @@ -848,6 +848,18 @@ function assignDefaults( result.expireTime ?? defaultDefault.expire } } + // This is the most dynamic cache life profile. + const secondsCacheLifeProfile = result.experimental.cacheLife['seconds'] + if ( + secondsCacheLifeProfile && + secondsCacheLifeProfile.stale === undefined + ) { + // We default this to whatever stale time you had configured for dynamic content. + // Since this is basically a dynamic cache life profile. + const dynamicStaleTime = result.experimental.staleTimes?.dynamic + secondsCacheLifeProfile.stale = + dynamicStaleTime ?? defaultConfig.experimental?.staleTimes?.dynamic + } } const userProvidedModularizeImports = result.modularizeImports diff --git a/packages/next/src/server/use-cache/cache-life.ts b/packages/next/src/server/use-cache/cache-life.ts index 9c08564e7a5ea..6a37578eafeb2 100644 --- a/packages/next/src/server/use-cache/cache-life.ts +++ b/packages/next/src/server/use-cache/cache-life.ts @@ -19,7 +19,16 @@ export type CacheLife = { // The default revalidates relatively frequently but doesn't expire to ensure it's always // able to serve fast results but by default doesn't hang. -type CacheLifeProfiles = string // This gets overridden by the next-types-plugin +// This gets overridden by the next-types-plugin +type CacheLifeProfiles = + | 'default' + | 'seconds' + | 'minutes' + | 'hours' + | 'days' + | 'weeks' + | 'max' + | string function validateCacheLife(profile: CacheLife) { if (profile.stale !== undefined) { From 51e1a249d11970de263c7ed2b43720d3c5136164 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Tue, 15 Oct 2024 23:34:00 +0200 Subject: [PATCH 3/3] Skip failing stack frames test (#71325) The test started [failing consistently](https://app.datadoghq.com/ci/test-runs?query=test_level%3Atest%20env%3Aci%20%40git.repository.id%3Agithub.com%2Fvercel%2Fnext.js%20%40test.service%3Anextjs%20%40test.status%3Afail%20%40test.name%3A%22ReactRefreshLogBox%20app%20default%20Should%20collapse%20bundler%20internal%20stack%20frames%22&agg_m=count&agg_m_source=base&agg_t=count¤tTab=overview&eventStack=&fromUser=false&index=citest&start=1728931189968&end=1729017589968&paused=false) after merging #71312. It's not reproducible locally, and needs more time to investigate. To unblock other PRs, we're skipping it for now. --- .../acceptance-app/ReactRefreshLogBox.test.ts | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts index 4189914331804..9a22ed84419a2 100644 --- a/test/development/acceptance-app/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance-app/ReactRefreshLogBox.test.ts @@ -1209,35 +1209,40 @@ export default function Home() { await cleanup() }) - test('Should collapse bundler internal stack frames', async () => { - const { session, browser, cleanup } = await sandbox( - next, - new Map([ - [ - 'app/utils.ts', - `throw new Error('utils error') + // TODO: Fails with Webpack since + // https://github.com/vercel/next.js/pull/71312, not reproducible locally, + // investigate why. + ;(isTurbopack ? test : test.skip)( + 'Should collapse bundler internal stack frames', + async () => { + const { session, browser, cleanup } = await sandbox( + next, + new Map([ + [ + 'app/utils.ts', + `throw new Error('utils error') export function foo(){}`, - ], - [ - 'app/page.js', - `"use client"; + ], + [ + 'app/page.js', + `"use client"; import { foo } from "./utils"; export default function Home() { foo(); return "hello"; }`, - ], - ]) - ) + ], + ]) + ) - await session.assertHasRedbox() + await session.assertHasRedbox() - let stack = next.normalizeTestDirContent( - await getRedboxCallStackCollapsed(browser) - ) - if (isTurbopack) { - expect(stack).toMatchInlineSnapshot(` + let stack = next.normalizeTestDirContent( + await getRedboxCallStackCollapsed(browser) + ) + if (isTurbopack) { + expect(stack).toMatchInlineSnapshot(` "app/utils.ts (1:7) @ [project]/app/utils.ts [app-client] (ecmascript) --- Next.js @@ -1249,8 +1254,8 @@ export default function Home() { --- React" `) - } else { - expect(stack).toMatchInlineSnapshot(` + } else { + expect(stack).toMatchInlineSnapshot(` "app/utils.ts (1:7) @ eval --- (app-pages-browser)/./app/utils.ts @@ -1268,8 +1273,9 @@ export default function Home() { --- React" `) - } + } - await cleanup() - }) + await cleanup() + } + ) })