Skip to content
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

Add support for "use cache" in route handlers #70897

Merged
merged 5 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next/src/build/flying-shuttle/stitch-builds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export async function stitchBuilds(
path.join(distDir, entryFile + '.nft.json')
)

if (type === 'app' && !entry.endsWith('/route')) {
if (type === 'app') {
const clientRefManifestFile = path.join(
'server',
type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,12 @@ export class ClientReferenceManifestPlugin {
manifestEntryFiles.push(entryName.replace(/\/page(\.[^/]+)?$/, '/page'))
}

// We also need to create manifests for route handler entrypoints to
// enable `'use cache'`.
if (/\/route$/.test(entryName)) {
manifestEntryFiles.push(entryName)
}

const groupName = entryNameToGroupName(entryName)
if (!manifestsPerGroup.has(groupName)) {
manifestsPerGroup.set(groupName, [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type { NodeFileTraceReasons } from 'next/dist/compiled/@vercel/nft'
import {
CLIENT_REFERENCE_MANIFEST,
TRACE_OUTPUT_VERSION,
UNDERSCORE_NOT_FOUND_ROUTE_ENTRY,
type CompilerNameValues,
} from '../../../shared/lib/constants'
import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
Expand Down Expand Up @@ -279,21 +278,17 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {

if (entrypoint.name.startsWith('app/')) {
// include the client reference manifest
const clientManifestsForPage =
entrypoint.name.endsWith('/page') ||
entrypoint.name === UNDERSCORE_NOT_FOUND_ROUTE_ENTRY
? nodePath.join(
outputPath,
outputPrefix,
entrypoint.name.replace(/%5F/g, '_') +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
)
: null
const clientManifestsForEntrypoint = nodePath.join(
outputPath,
outputPrefix,
entrypoint.name.replace(/%5F/g, '_') +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
)

if (clientManifestsForPage !== null) {
entryFiles.add(clientManifestsForPage)
if (clientManifestsForEntrypoint !== null) {
entryFiles.add(clientManifestsForEntrypoint)
}
}

Expand Down
7 changes: 1 addition & 6 deletions packages/next/src/build/webpack/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import type {
Module,
ModuleGraph,
} from 'webpack'
import { isAppRouteRoute } from '../../lib/is-app-route-route'
import type { ModuleGraphConnection } from 'webpack'

export function traverseModules(
Expand Down Expand Up @@ -48,11 +47,7 @@ export function forEachEntryModule(
) {
for (const [name, entry] of compilation.entries.entries()) {
// Skip for entries under pages/
if (
name.startsWith('pages/') ||
// Skip for route.js entries
(name.startsWith('app/') && isAppRouteRoute(name))
) {
if (name.startsWith('pages/')) {
continue
}

Expand Down
7 changes: 1 addition & 6 deletions packages/next/src/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
REACT_LOADABLE_MANIFEST,
CLIENT_REFERENCE_MANIFEST,
SERVER_REFERENCE_MANIFEST,
UNDERSCORE_NOT_FOUND_ROUTE,
} from '../shared/lib/constants'
import { join } from 'path'
import { requirePage } from './require'
Expand Down Expand Up @@ -140,10 +139,6 @@ async function loadComponentsImpl<N = any>({
])
}

// Make sure to avoid loading the manifest for Route Handlers
const hasClientManifest =
isAppPath && (page.endsWith('/page') || page === UNDERSCORE_NOT_FOUND_ROUTE)

// Load the manifest files first
const [
buildManifest,
Expand All @@ -155,7 +150,7 @@ async function loadComponentsImpl<N = any>({
loadManifestWithRetries<ReactLoadableManifest>(
join(distDir, REACT_LOADABLE_MANIFEST)
),
hasClientManifest
isAppPath
? loadClientReferenceManifest(
join(
distDir,
Expand Down
81 changes: 53 additions & 28 deletions test/e2e/app-dir/app-static/app-static.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ import stripAnsi from 'strip-ansi'
const glob = promisify(globOrig)

describe('app-dir static/dynamic handling', () => {
const { next, isNextDev, isNextStart, isNextDeploy } = nextTestSetup({
files: __dirname,
env: {
NEXT_DEBUG_BUILD: '1',
...(process.env.CUSTOM_CACHE_HANDLER
? {
CUSTOM_CACHE_HANDLER: process.env.CUSTOM_CACHE_HANDLER,
}
: {}),
},
})
const { next, isNextDev, isNextStart, isNextDeploy, isTurbopack } =
nextTestSetup({
files: __dirname,
env: {
NEXT_DEBUG_BUILD: '1',
...(process.env.CUSTOM_CACHE_HANDLER
? {
CUSTOM_CACHE_HANDLER: process.env.CUSTOM_CACHE_HANDLER,
}
: {}),
},
})

let prerenderManifest
let buildCliOutputIndex = 0
Expand Down Expand Up @@ -447,30 +448,36 @@ describe('app-dir static/dynamic handling', () => {
})

if (!isNextDev && !process.env.CUSTOM_CACHE_HANDLER) {
it('should properly revalidate a route handler that triggers dynamic usage with force-static', async () => {
// wait for the revalidation period
let res = await next.fetch('/route-handler/no-store-force-static')
// TODO: Temporarily disabling this test for Turbopack. The test is failing
// quite often (see https://app.datadoghq.com/ci/test-runs?query=test_level%3Atest%20env%3Aci%20%40git.repository.id%3Agh.neting.cc%2Fvercel%2Fnext.js%20%40test.service%3Anextjs%20%40test.status%3Afail%20%40test.name%3A%22app-dir%20static%2Fdynamic%20handling%20should%20properly%20revalidate%20a%20route%20handler%20that%20triggers%20dynamic%20usage%20with%20force-static%22&agg_m=count&agg_m_source=base&agg_t=count&currentTab=overview&eventStack=&fromUser=false&index=citest&start=1720993078523&end=1728769078523&paused=false).
// Since this is also reproducible when manually recreating the scenario, it
// might actually be a bug with ISR, which needs to be investigated.
if (!isTurbopack) {
it('should properly revalidate a route handler that triggers dynamic usage with force-static', async () => {
// wait for the revalidation period
let res = await next.fetch('/route-handler/no-store-force-static')

let data = await res.json()
// grab the initial timestamp
const initialTimestamp = data.now
let data = await res.json()
// grab the initial timestamp
const initialTimestamp = data.now

// confirm its cached still
res = await next.fetch('/route-handler/no-store-force-static')
// confirm its cached still
res = await next.fetch('/route-handler/no-store-force-static')

data = await res.json()
data = await res.json()

expect(data.now).toBe(initialTimestamp)
expect(data.now).toBe(initialTimestamp)

// wait for the revalidation time
await waitFor(3000)
// wait for the revalidation time
await waitFor(3000)

// verify fresh data
res = await next.fetch('/route-handler/no-store-force-static')
data = await res.json()
// verify fresh data
res = await next.fetch('/route-handler/no-store-force-static')
data = await res.json()

expect(data.now).not.toBe(initialTimestamp)
})
expect(data.now).not.toBe(initialTimestamp)
})
}
}

if (!process.env.CUSTOM_CACHE_HANDLER) {
Expand Down Expand Up @@ -754,6 +761,7 @@ describe('app-dir static/dynamic handling', () => {
"(new)/custom/page.js",
"(new)/custom/page_client-reference-manifest.js",
"(new)/default-config-fetch/api/route.js",
"(new)/default-config-fetch/api/route_client-reference-manifest.js",
"(new)/default-config-fetch/page.js",
"(new)/default-config-fetch/page_client-reference-manifest.js",
"(new)/no-config-fetch/page.js",
Expand All @@ -763,11 +771,15 @@ describe('app-dir static/dynamic handling', () => {
"_not-found/page.js",
"_not-found/page_client-reference-manifest.js",
"api/draft-mode/route.js",
"api/draft-mode/route_client-reference-manifest.js",
"api/large-data/route.js",
"api/large-data/route_client-reference-manifest.js",
"api/revalidate-path-edge/route.js",
"api/revalidate-path-node/route.js",
"api/revalidate-path-node/route_client-reference-manifest.js",
"api/revalidate-tag-edge/route.js",
"api/revalidate-tag-node/route.js",
"api/revalidate-tag-node/route_client-reference-manifest.js",
"articles/[slug]/page.js",
"articles/[slug]/page_client-reference-manifest.js",
"articles/works.html",
Expand Down Expand Up @@ -819,15 +831,19 @@ describe('app-dir static/dynamic handling', () => {
"force-dynamic-fetch-cache/default-cache/page.js",
"force-dynamic-fetch-cache/default-cache/page_client-reference-manifest.js",
"force-dynamic-fetch-cache/default-cache/route/route.js",
"force-dynamic-fetch-cache/default-cache/route/route_client-reference-manifest.js",
"force-dynamic-fetch-cache/force-cache/page.js",
"force-dynamic-fetch-cache/force-cache/page_client-reference-manifest.js",
"force-dynamic-fetch-cache/force-cache/route/route.js",
"force-dynamic-fetch-cache/force-cache/route/route_client-reference-manifest.js",
"force-dynamic-fetch-cache/no-fetch-cache/page.js",
"force-dynamic-fetch-cache/no-fetch-cache/page_client-reference-manifest.js",
"force-dynamic-fetch-cache/no-fetch-cache/route/route.js",
"force-dynamic-fetch-cache/no-fetch-cache/route/route_client-reference-manifest.js",
"force-dynamic-fetch-cache/with-fetch-cache/page.js",
"force-dynamic-fetch-cache/with-fetch-cache/page_client-reference-manifest.js",
"force-dynamic-fetch-cache/with-fetch-cache/route/route.js",
"force-dynamic-fetch-cache/with-fetch-cache/route/route_client-reference-manifest.js",
"force-dynamic-no-prerender/[id]/page.js",
"force-dynamic-no-prerender/[id]/page_client-reference-manifest.js",
"force-dynamic-prerender/[slug]/page.js",
Expand Down Expand Up @@ -920,11 +936,17 @@ describe('app-dir static/dynamic handling', () => {
"response-url/page_client-reference-manifest.js",
"route-handler-edge/revalidate-360/route.js",
"route-handler/no-store-force-static/route.js",
"route-handler/no-store-force-static/route_client-reference-manifest.js",
"route-handler/no-store/route.js",
"route-handler/no-store/route_client-reference-manifest.js",
"route-handler/post/route.js",
"route-handler/post/route_client-reference-manifest.js",
"route-handler/revalidate-360-isr/route.js",
"route-handler/revalidate-360-isr/route_client-reference-manifest.js",
"route-handler/revalidate-360/route.js",
"route-handler/revalidate-360/route_client-reference-manifest.js",
"route-handler/static-cookies/route.js",
"route-handler/static-cookies/route_client-reference-manifest.js",
"specify-new-tags/one-tag/page.js",
"specify-new-tags/one-tag/page_client-reference-manifest.js",
"specify-new-tags/two-tags/page.js",
Expand All @@ -949,6 +971,7 @@ describe('app-dir static/dynamic handling', () => {
"stale-cache-serving/app-page/page.js",
"stale-cache-serving/app-page/page_client-reference-manifest.js",
"stale-cache-serving/route-handler/route.js",
"stale-cache-serving/route-handler/route_client-reference-manifest.js",
"static-to-dynamic-error-forced/[id]/page.js",
"static-to-dynamic-error-forced/[id]/page_client-reference-manifest.js",
"static-to-dynamic-error/[id]/page.js",
Expand Down Expand Up @@ -996,7 +1019,9 @@ describe('app-dir static/dynamic handling', () => {
"variable-revalidate/authorization/page.js",
"variable-revalidate/authorization/page_client-reference-manifest.js",
"variable-revalidate/authorization/route-cookies/route.js",
"variable-revalidate/authorization/route-cookies/route_client-reference-manifest.js",
"variable-revalidate/authorization/route-request/route.js",
"variable-revalidate/authorization/route-request/route_client-reference-manifest.js",
"variable-revalidate/cookie.html",
"variable-revalidate/cookie.rsc",
"variable-revalidate/cookie/page.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('Node Extensions', () => {
expect($('li').length).toBe(2)
})

it.skip('should not error when accessing routes that use Math.random() in App Router', async () => {
it('should not error when accessing routes that use Math.random() in App Router', async () => {
let res, body

res = await next.fetch('/app/prerendered/uncached/api')
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/app-dir/use-cache/app/api/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' },
})
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/use-cache/use-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,11 @@ describe('use-cache', () => {
)
}
})

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

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