Skip to content

Commit

Permalink
For route handlers, call loadComponents also during next build
Browse files Browse the repository at this point in the history
During `next dev` for route handlers, `loadComponents` is called to load
the route module. This also has the (for now still expected) side-effect
that the reference manifests singleton is set, which we need to make
`'use cache'` work in router handlers. Until now, `loadComponents` was
not called during `next build` for router handlers, and the module was
loaded separately with `RouteModuleLoader.load`.

With this PR, we are aligning both modes, so that it's always ensured
that the reference manifests singleton is set.

The end goal is to avoid the global singleton completely, but it's still
unclear how to achive this because of the requirements and constrains
that are documented in #62437.
  • Loading branch information
unstubbable committed Oct 12, 2024
1 parent 7e34307 commit 0e410ee
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 17 deletions.
11 changes: 1 addition & 10 deletions packages/next/src/export/routes/app-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ import type AppRouteRouteModule from '../../server/route-modules/app-route/modul
import type { AppRouteRouteHandlerContext } from '../../server/route-modules/app-route/module'
import type { IncrementalCache } from '../../server/lib/incremental-cache'

import { join } from 'path'
import {
INFINITE_CACHE,
NEXT_BODY_SUFFIX,
NEXT_CACHE_TAGS_HEADER,
NEXT_META_SUFFIX,
} from '../../lib/constants'
import { NodeNextRequest } from '../../server/base-http/node'
import { RouteModuleLoader } from '../../server/lib/module-loader/route-module-loader'
import {
NextRequestAdapter,
signalFromNodeResponse,
Expand All @@ -22,7 +20,6 @@ import type {
MockedResponse,
} from '../../server/lib/mock-request'
import { isDynamicUsageError } from '../helpers/is-dynamic-usage-error'
import { SERVER_DIRECTORY } from '../../shared/lib/constants'
import { hasNextSupport } from '../../server/ci-info'
import { isStaticGenEnabled } from '../../server/route-modules/app-route/helpers/is-static-gen-enabled'
import type { ExperimentalConfig } from '../../server/config-shared'
Expand All @@ -40,8 +37,8 @@ export async function exportAppRoute(
res: MockedResponse,
params: Params | undefined,
page: string,
module: AppRouteRouteModule,
incrementalCache: IncrementalCache | undefined,
distDir: string,
htmlFilepath: string,
fileWriter: FileWriter,
experimental: Required<Pick<ExperimentalConfig, 'after' | 'dynamicIO'>>,
Expand Down Expand Up @@ -86,13 +83,7 @@ export async function exportAppRoute(
context.renderOpts.isRevalidate = true
}

// This is a route handler, which means it has it's handler in the
// bundled file already, we should just use that.
const filename = join(distDir, SERVER_DIRECTORY, 'app', page)

try {
// Route module loading and handling.
const module = await RouteModuleLoader.load<AppRouteRouteModule>(filename)
const userland = module.userland
// we don't bail from the static optimization for
// metadata routes
Expand Down
15 changes: 8 additions & 7 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
} from '../server/request/fallback-params'
import { needsExperimentalReact } from '../lib/needs-experimental-react'
import { runWithCacheScope } from '../server/async-storage/cache-scope'
import type { AppRouteRouteModule } from '../server/route-modules/app-route/module.compiled'

const envConfig = require('../shared/lib/runtime-config.external')

Expand Down Expand Up @@ -235,28 +236,28 @@ async function exportPageImpl(

await fs.mkdir(baseDir, { recursive: true })

const components = await loadComponents({
distDir,
page,
isAppPath: isAppDir,
})

// Handle App Routes.
if (isAppDir && isAppRouteRoute(page)) {
return await exportAppRoute(
req,
res,
params,
page,
components.routeModule as AppRouteRouteModule,
input.renderOpts.incrementalCache,
distDir,
htmlFilepath,
fileWriter,
input.renderOpts.experimental,
input.renderOpts.buildId
)
}

const components = await loadComponents({
distDir,
page,
isAppPath: isAppDir,
})

const renderOpts: WorkerRenderOpts = {
...components,
...input.renderOpts,
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/app-dir/use-cache-route-handler-only/app/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
async function getCachedRandom() {
'use cache'
return Math.random()
}

export async function GET() {
const rand1 = await getCachedRandom()
// TODO: Remove this extra micro task when bug in use cache wrapper is fixed.
await Promise.resolve()
const rand2 = await getCachedRandom()

const response = JSON.stringify({ rand1, rand2 })

return new Response(response, {
headers: { 'content-type': 'application/json' },
})
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/use-cache-route-handler-only/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {
experimental: {
dynamicIO: true,
},
}

module.exports = nextConfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* 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({
files: __dirname,
})

const itSkipTurbopack = isTurbopack ? it.skip : it

itSkipTurbopack('should cache results in route handlers', async () => {
const response = await next.fetch('/')
const { rand1, rand2 } = await response.json()

expect(rand1).toEqual(rand2)
})
})

0 comments on commit 0e410ee

Please sign in to comment.