-
Notifications
You must be signed in to change notification settings - Fork 30k
WIP: perf: Don't inject the entire nextConfig in edge templates #86630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: canary
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,10 +81,11 @@ pub struct NextConfig { | |
| config_file: Option<RcStr>, | ||
| config_file_name: RcStr, | ||
|
|
||
| cache_life: Option<JsonValue>, | ||
| /// In-memory cache size in bytes. | ||
| /// | ||
| /// If `cache_max_memory_size: 0` disables in-memory caching. | ||
| cache_max_memory_size: Option<f64>, | ||
| cache_max_memory_size: Option<JsonValue>, | ||
| /// custom path to a cache handler to use | ||
| cache_handler: Option<RcStr>, | ||
| cache_handlers: Option<FxIndexMap<RcStr, RcStr>>, | ||
|
|
@@ -835,7 +836,6 @@ pub struct ExperimentalConfig { | |
| adjust_font_fallbacks_with_size_adjust: Option<bool>, | ||
| after: Option<bool>, | ||
| app_document_preloading: Option<bool>, | ||
| cache_life: Option<FxIndexMap<String, CacheLifeProfile>>, | ||
| case_sensitive_routes: Option<bool>, | ||
| cpus: Option<f64>, | ||
| cra_compat: Option<bool>, | ||
|
|
@@ -908,63 +908,6 @@ pub struct ExperimentalConfig { | |
| devtool_segment_explorer: Option<bool>, | ||
| } | ||
|
|
||
| #[derive( | ||
| Clone, Debug, PartialEq, Serialize, Deserialize, TraceRawVcs, NonLocalValue, OperationValue, | ||
| )] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct CacheLifeProfile { | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub stale: Option<u32>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub revalidate: Option<u32>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub expire: Option<u32>, | ||
| } | ||
|
Comment on lines
-915
to
-922
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't actually care what the contents of |
||
|
|
||
| #[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<ExperimentalConfig, _> = serde_json::from_value(json); | ||
|
|
||
| assert!( | ||
| result.is_err(), | ||
| "Deserialization should fail due to invalid 'stale' value type" | ||
| ); | ||
| } | ||
|
|
||
| #[derive( | ||
| Clone, Debug, PartialEq, Eq, Serialize, Deserialize, TraceRawVcs, NonLocalValue, OperationValue, | ||
| )] | ||
|
|
@@ -1312,6 +1255,16 @@ impl NextConfig { | |
| Vc::cell(self.base_path.clone()) | ||
| } | ||
|
|
||
| #[turbo_tasks::function] | ||
| pub fn cache_life(&self) -> Vc<OptionJsonValue> { | ||
| Vc::cell(self.cache_life.clone()) | ||
| } | ||
|
|
||
| #[turbo_tasks::function] | ||
| pub fn cache_max_memory_size(&self) -> Vc<OptionJsonValue> { | ||
| Vc::cell(self.cache_max_memory_size.clone()) | ||
| } | ||
|
|
||
| #[turbo_tasks::function] | ||
| pub fn cache_handler(&self, project_path: FileSystemPath) -> Result<Vc<OptionFileSystemPath>> { | ||
| if let Some(handler) = &self.cache_handler { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ import { IncrementalCache } from '../../server/lib/incremental-cache' | |
| import * as pageMod from 'VAR_USERLAND' | ||
|
|
||
| import type { RequestData } from '../../server/web/types' | ||
| import type { NextConfigComplete } from '../../server/config-shared' | ||
| import { setReferenceManifestsSingleton } from '../../server/app-render/encryption-utils' | ||
| import { createServerModuleMap } from '../../server/app-render/action-utils' | ||
| import { initializeCacheHandlers } from '../../server/use-cache/handlers' | ||
|
|
@@ -27,12 +26,12 @@ import { checkIsOnDemandRevalidate } from '../../server/api-utils' | |
| import { CloseController } from '../../server/web/web-on-close' | ||
|
|
||
| declare const incrementalCacheHandler: any | ||
| declare const nextConfig: NextConfigComplete | ||
| declare const cacheMaxMemorySize: number | ||
| // OPTIONAL_IMPORT:incrementalCacheHandler | ||
| // INJECT:nextConfig | ||
| // INJECT:cacheMaxMemorySize | ||
|
|
||
| // Initialize the cache handlers interface. | ||
| initializeCacheHandlers(nextConfig.cacheMaxMemorySize) | ||
| initializeCacheHandlers(cacheMaxMemorySize) | ||
|
|
||
| const maybeJSONParse = (str?: string) => (str ? JSON.parse(str) : undefined) | ||
|
|
||
|
|
@@ -77,6 +76,7 @@ async function requestHandler( | |
| const { | ||
| query, | ||
| params, | ||
| nextConfig, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to edge-ssr.ts, the edge-ssr-app.ts template extracts View Details📝 Patch Detailsdiff --git a/packages/next/src/build/entries.ts b/packages/next/src/build/entries.ts
index e48f0573b4..320561a8d7 100644
--- a/packages/next/src/build/entries.ts
+++ b/packages/next/src/build/entries.ts
@@ -680,6 +680,7 @@ export function getEdgeServerEntry(opts: {
isServerComponent: opts.isServerComponent,
page: opts.page,
cacheMaxMemorySize: JSON.stringify(opts.config.cacheMaxMemorySize),
+ nextConfig: Buffer.from(JSON.stringify(opts.config)).toString('base64'),
pagesType: opts.pagesType,
appDirLoader: Buffer.from(opts.appDirLoader || '').toString('base64'),
sriEnabled: !opts.isDev && !!opts.config.experimental.sri?.algorithm,
diff --git a/packages/next/src/build/templates/edge-ssr-app.ts b/packages/next/src/build/templates/edge-ssr-app.ts
index ba39c22871..b8d5a8c74a 100644
--- a/packages/next/src/build/templates/edge-ssr-app.ts
+++ b/packages/next/src/build/templates/edge-ssr-app.ts
@@ -24,11 +24,14 @@ import { interopDefault } from '../../lib/interop-default'
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'
import { checkIsOnDemandRevalidate } from '../../server/api-utils'
import { CloseController } from '../../server/web/web-on-close'
+import type { NextConfigComplete } from '../../server/config-shared'
declare const incrementalCacheHandler: any
declare const cacheMaxMemorySize: number
+declare const nextConfig: NextConfigComplete
// OPTIONAL_IMPORT:incrementalCacheHandler
// INJECT:cacheMaxMemorySize
+// INJECT:nextConfig
// Initialize the cache handlers interface.
initializeCacheHandlers(cacheMaxMemorySize)
@@ -76,7 +79,6 @@ async function requestHandler(
const {
query,
params,
- nextConfig,
buildId,
buildManifest,
prerenderManifest,
diff --git a/packages/next/src/build/templates/edge-ssr.ts b/packages/next/src/build/templates/edge-ssr.ts
index ad3fdd40c9..ceebc51115 100644
--- a/packages/next/src/build/templates/edge-ssr.ts
+++ b/packages/next/src/build/templates/edge-ssr.ts
@@ -25,13 +25,16 @@ import type { RenderResultMetadata } from '../../server/render-result'
import { getTracer, SpanKind, type Span } from '../../server/lib/trace/tracer'
import { BaseServerSpan } from '../../server/lib/trace/constants'
import { HTML_CONTENT_TYPE_HEADER } from '../../lib/constants'
+import type { NextConfigComplete } from '../../server/config-shared'
// injected by the loader afterwards.
declare const cacheMaxMemorySize: number
+declare const nextConfig: NextConfigComplete
declare const pageRouteModuleOptions: any
declare const errorRouteModuleOptions: any
declare const user500RouteModuleOptions: any
// INJECT:cacheMaxMemorySize
+// INJECT:nextConfig
// INJECT:pageRouteModuleOptions
// INJECT:errorRouteModuleOptions
// INJECT:user500RouteModuleOptions
@@ -102,7 +105,6 @@ async function requestHandler(
const {
query,
params,
- nextConfig,
buildId,
isNextDataRequest,
buildManifest,
diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts
index 85db21c2d5..637e2fa106 100644
--- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts
+++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/index.ts
@@ -20,6 +20,7 @@ export type EdgeSSRLoaderQuery = {
isServerComponent: boolean
page: string
cacheMaxMemorySize: string
+ nextConfig: string
appDirLoader?: string
pagesType: PAGE_TYPES
sriEnabled: boolean
@@ -74,6 +75,7 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
absoluteErrorPath,
isServerComponent,
cacheMaxMemorySize: cacheMaxMemorySizeStringified,
+ nextConfig: nextConfigBase64,
appDirLoader: appDirLoaderBase64,
pagesType,
cacheHandler,
@@ -94,6 +96,10 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
Buffer.from(middlewareConfigBase64, 'base64').toString()
)
+ const nextConfig = JSON.parse(
+ Buffer.from(nextConfigBase64 || '', 'base64').toString()
+ )
+
const appDirLoader = Buffer.from(
appDirLoaderBase64 || '',
'base64'
@@ -158,6 +164,7 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
},
{
cacheMaxMemorySize: cacheMaxMemorySizeStringified,
+ nextConfig: JSON.stringify(nextConfig),
},
{
incrementalCacheHandler: cacheHandler ?? null,
@@ -175,6 +182,7 @@ const edgeSSRLoader: webpack.LoaderDefinitionFunction<EdgeSSRLoaderQuery> =
},
{
cacheMaxMemorySize: cacheMaxMemorySizeStringified,
+ nextConfig: JSON.stringify(nextConfig),
pageRouteModuleOptions: JSON.stringify(getRouteModuleOptions(page)),
errorRouteModuleOptions: JSON.stringify(
getRouteModuleOptions('/_error')
AnalysisMissing nextConfig injection in edge SSR templates causes property access failuresWhat fails: The edge SSR templates ( How to reproduce: Build and deploy a Next.js application using the edge runtime with app router features that depend on nextConfig properties: # Build with edge runtime enabled
npm run build
# Any request to an app route at edge runtime will receive incorrect config values:
# - nextConfig.assetPrefix → undefined
# - nextConfig.experimental.taint → undefined
# - nextConfig.cacheLife → undefined
# - nextConfig.images → undefined
# etc.Result: WIP commit e20fb2a ("perf: Don't inject the entire nextConfig in edge templates") removed the The affected code paths in edge-ssr-app.ts (lines 141-163) and edge-ssr.ts (lines 142-157) set renderContext properties using these undefined values:
Expected: All nextConfig properties should have their actual values from next.config.js, not undefined. The edge template bundle should include the full nextConfig via proper injection (as in commit predecessor). Fix: Restored the
|
||
| buildId, | ||
| buildManifest, | ||
| prerenderManifest, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example of why this was a bad idea: This config was moved out of experimental, but Turbopack's config was never updated, so
edge-app-route.ts was probably reading the wrong value when using Turbopack...