From 0e410eec7372e503151d350c04a288ee677589cd Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Sat, 12 Oct 2024 23:28:10 +0200 Subject: [PATCH] For route handlers, call `loadComponents` also during `next build` 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. --- packages/next/src/export/routes/app-route.ts | 11 +---------- packages/next/src/export/worker.ts | 15 ++++++++------- .../use-cache-route-handler-only/app/route.ts | 17 +++++++++++++++++ .../next.config.js | 10 ++++++++++ .../use-cache-route-handler-only.test.ts | 19 +++++++++++++++++++ 5 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 test/e2e/app-dir/use-cache-route-handler-only/app/route.ts create mode 100644 test/e2e/app-dir/use-cache-route-handler-only/next.config.js create mode 100644 test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts diff --git a/packages/next/src/export/routes/app-route.ts b/packages/next/src/export/routes/app-route.ts index db4043466c127f..8f6cc8ae7bd7bc 100644 --- a/packages/next/src/export/routes/app-route.ts +++ b/packages/next/src/export/routes/app-route.ts @@ -3,7 +3,6 @@ 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, @@ -11,7 +10,6 @@ import { 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, @@ -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' @@ -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>, @@ -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(filename) const userland = module.userland // we don't bail from the static optimization for // metadata routes diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 2503188b8cbbfc..76a7ff8cf29701 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -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') @@ -235,6 +236,12 @@ 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( @@ -242,8 +249,8 @@ async function exportPageImpl( res, params, page, + components.routeModule as AppRouteRouteModule, input.renderOpts.incrementalCache, - distDir, htmlFilepath, fileWriter, input.renderOpts.experimental, @@ -251,12 +258,6 @@ async function exportPageImpl( ) } - const components = await loadComponents({ - distDir, - page, - isAppPath: isAppDir, - }) - const renderOpts: WorkerRenderOpts = { ...components, ...input.renderOpts, diff --git a/test/e2e/app-dir/use-cache-route-handler-only/app/route.ts b/test/e2e/app-dir/use-cache-route-handler-only/app/route.ts new file mode 100644 index 00000000000000..d52bf97fa2c8d9 --- /dev/null +++ b/test/e2e/app-dir/use-cache-route-handler-only/app/route.ts @@ -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' }, + }) +} diff --git a/test/e2e/app-dir/use-cache-route-handler-only/next.config.js b/test/e2e/app-dir/use-cache-route-handler-only/next.config.js new file mode 100644 index 00000000000000..ac4afcf4321968 --- /dev/null +++ b/test/e2e/app-dir/use-cache-route-handler-only/next.config.js @@ -0,0 +1,10 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + experimental: { + dynamicIO: true, + }, +} + +module.exports = nextConfig 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 new file mode 100644 index 00000000000000..6b44fc35d10076 --- /dev/null +++ b/test/e2e/app-dir/use-cache-route-handler-only/use-cache-route-handler-only.test.ts @@ -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) + }) +})