Skip to content

Commit

Permalink
Fix edge preview props are not matched with cookie (#67779)
Browse files Browse the repository at this point in the history
Chnage the way of preview props injection to edge runtime, directly read
from env vars of preview props instead of writing to and reading from
prerender manifest.js

Previously we're trying to make these preview props values become
deterministic that we can replace in edge deployment pipeline in #64521

But the way of serializing process env vars in edge runtime is not
correct. They'll remain as string "process.env.xxx" in the manifest and
also after consumed. This PR fixes that behavior, instead of writing it
into manifest, alwyas consuming from process.env.var directly.

I created a shared util to access the preview props of edge runtime
across all the templates.

On draft provider side, we still need to handle dev mode case when
preview id is `development-id`, but we already have the cookie, it
cannot be aligned with the preview id. So we do a fallback check for dev
mode if the cookie is present and preview id is `development-id` then we
still treat it as draft mode is enabled.

Fixes #52080
Fixes #67075

Closes NEXT-3541
  • Loading branch information
huozhi authored and lubieowoce committed Sep 3, 2024
1 parent efaa61e commit 8c6608e
Show file tree
Hide file tree
Showing 15 changed files with 147 additions and 37 deletions.
10 changes: 4 additions & 6 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,10 @@ async function writeEdgePartialPrerenderManifest(
dynamicRoutes: {},
notFoundRoutes: [],
version: manifest.version,
preview: {
previewModeId: 'process.env.__NEXT_PREVIEW_MODE_ID',
previewModeSigningKey: 'process.env.__NEXT_PREVIEW_MODE_SIGNING_KEY',
previewModeEncryptionKey:
'process.env.__NEXT_PREVIEW_MODE_ENCRYPTION_KEY',
},
// Preview props are inlined in the code with dynamic env vars,
// During edge runtime build:
// - local: env vars will be injected through edge-runtime as runtime env vars
// - deployment: env vars will be replaced by edge build pipeline as inline values
}
await writeFileUtf8(
path.join(distDir, PRERENDER_MANIFEST.replace(/\.json$/, '.js')),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { internal_getCurrentFunctionWaitUntil } from '../../../../server/web/int
import type { PAGE_TYPES } from '../../../../lib/page-types'
import type { NextRequestHint } from '../../../../server/web/adapter'
import type { DeepReadonly } from '../../../../shared/lib/deep-readonly'
import { getEdgePreviewProps } from '../../../../server/web/get-edge-preview-props'

export function getRender({
dev,
Expand Down Expand Up @@ -88,7 +89,12 @@ export function getRender({
page,
pathname: isAppPath ? normalizeAppPath(page) : page,
pagesType,
prerenderManifest,
prerenderManifest: prerenderManifest
? {
...prerenderManifest,
preview: getEdgePreviewProps(),
}
: undefined,
interceptionRouteRewrites,
extendRenderOpts: {
buildId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ export class DraftModeProvider {
!isOnDemandRevalidate &&
cookieValue &&
previewProps &&
cookieValue === previewProps.previewModeId
(cookieValue === previewProps.previewModeId ||
// In dev mode, the cookie can be actual hash value preview id but the preview props can still be `development-id`.
(process.env.NODE_ENV !== 'production' &&
previewProps.previewModeId === 'development-id'))
)

this._previewModeId = previewProps?.previewModeId
Expand Down
5 changes: 2 additions & 3 deletions packages/next/src/server/web-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type { Rewrite } from '../lib/load-custom-routes'
import { buildCustomRoute } from '../lib/build-custom-route'
import { UNDERSCORE_NOT_FOUND_ROUTE } from '../api/constants'
import type { DeepReadonly } from '../shared/lib/deep-readonly'
import { getEdgePreviewProps } from './web/get-edge-preview-props'

interface WebServerOptions extends Options {
webServerConfig: {
Expand Down Expand Up @@ -140,9 +141,7 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
routes: {},
dynamicRoutes: {},
notFoundRoutes: [],
preview: {
previewModeId: 'development-id',
} as any, // `preview` is special case read in next-dev-server
preview: getEdgePreviewProps(),
}
}
return prerenderManifest
Expand Down
16 changes: 3 additions & 13 deletions packages/next/src/server/web/adapter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { RequestData, FetchEventResult } from './types'
import type { RequestInit } from './spec-extension/request'
import type { PrerenderManifest } from '../../build'
import { PageSignatureError } from './error'
import { fromNodeOutgoingHttpHeaders } from './utils'
import { NextFetchEvent } from './spec-extension/fetch-event'
Expand All @@ -19,6 +18,7 @@ import { requestAsyncStorage } from '../../client/components/request-async-stora
import { getTracer } from '../lib/trace/tracer'
import type { TextMapGetter } from 'next/dist/compiled/@opentelemetry/api'
import { MiddlewareSpan } from '../lib/trace/constants'
import { getEdgePreviewProps } from './get-edge-preview-props'

export class NextRequestHint extends NextRequest {
sourcePage: string
Expand Down Expand Up @@ -90,10 +90,6 @@ export async function adapter(

// TODO-APP: use explicit marker for this
const isEdgeRendering = typeof self.__BUILD_MANIFEST !== 'undefined'
const prerenderManifest: PrerenderManifest | undefined =
typeof self.__PRERENDER_MANIFEST === 'string'
? JSON.parse(self.__PRERENDER_MANIFEST)
: undefined

params.request.url = normalizeRscURL(params.request.url)

Expand Down Expand Up @@ -197,9 +193,7 @@ export async function adapter(
routes: {},
dynamicRoutes: {},
notFoundRoutes: [],
preview: {
previewModeId: 'development-id',
} as any, // `preview` is special case read in next-dev-server
preview: getEdgePreviewProps(),
}
},
})
Expand Down Expand Up @@ -233,11 +227,7 @@ export async function adapter(
cookiesFromResponse = cookies
},
// @ts-expect-error: TODO: investigate why previewProps isn't on RenderOpts
previewProps: prerenderManifest?.preview || {
previewModeId: 'development-id',
previewModeEncryptionKey: '',
previewModeSigningKey: '',
},
previewProps: getEdgePreviewProps(),
},
},
() => params.handler(request, event)
Expand Down
13 changes: 3 additions & 10 deletions packages/next/src/server/web/edge-route-module-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type {
AppRouteRouteHandlerContext,
AppRouteRouteModule,
} from '../future/route-modules/app-route/module'
import type { PrerenderManifest } from '../../build'

import './globals'

Expand All @@ -14,6 +13,7 @@ import type { NextFetchEvent } from './spec-extension/fetch-event'
import { internal_getCurrentFunctionWaitUntil } from './internal-edge-wait-until'
import { getUtils } from '../server-utils'
import { searchParamsToUrlQuery } from '../../shared/lib/router/utils/querystring'
import { getEdgePreviewProps } from './get-edge-preview-props'

type WrapOptions = Partial<Pick<AdapterOptions, 'page'>>

Expand Down Expand Up @@ -82,10 +82,7 @@ export class EdgeRouteModuleWrapper {
searchParamsToUrlQuery(request.nextUrl.searchParams)
)

const prerenderManifest: PrerenderManifest | undefined =
typeof self.__PRERENDER_MANIFEST === 'string'
? JSON.parse(self.__PRERENDER_MANIFEST)
: undefined
const previewProps = getEdgePreviewProps()

// Create the context for the handler. This contains the params from the
// match (if any).
Expand All @@ -95,11 +92,7 @@ export class EdgeRouteModuleWrapper {
version: 4,
routes: {},
dynamicRoutes: {},
preview: prerenderManifest?.preview || {
previewModeEncryptionKey: '',
previewModeId: 'development-id',
previewModeSigningKey: '',
},
preview: previewProps,
notFoundRoutes: [],
},
renderOpts: {
Expand Down
16 changes: 16 additions & 0 deletions packages/next/src/server/web/get-edge-preview-props.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* In edge runtime, these props directly accessed from environment variables.
* - local: env vars will be injected through edge-runtime as runtime env vars
* - deployment: env vars will be replaced by edge build pipeline
*/
export function getEdgePreviewProps() {
return {
previewModeId:
process.env.NODE_ENV === 'production'
? process.env.__NEXT_PREVIEW_MODE_ID!
: 'development-id',
previewModeSigningKey: process.env.__NEXT_PREVIEW_MODE_SIGNING_KEY || '',
previewModeEncryptionKey:
process.env.__NEXT_PREVIEW_MODE_ENCRYPTION_KEY || '',
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { draftMode } from 'next/headers'

export async function GET(request: Request) {
draftMode().disable()
return new Response('Draft mode is disabled')
}
23 changes: 23 additions & 0 deletions test/e2e/app-dir/draft-mode-middleware/app/api/draft/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// route handler with secret and slug
import { draftMode } from 'next/headers'
import { redirect } from 'next/navigation'

// Preview URL: localhost:3000/api/draft?secret=secret-token&slug=preview-page

export async function GET(request: Request) {
// Parse query string parameters
const { searchParams } = new URL(request.url)
const secret = searchParams.get('secret')
const slug = searchParams.get('slug')

// Check the secret and next parameters
if (secret !== 'secret-token' || !slug) {
return new Response('Invalid token', { status: 401 })
}

// Enable Draft Mode by setting the cookie
draftMode().enable()

// Redirect to the path
redirect(`/${slug}`)
}
16 changes: 16 additions & 0 deletions test/e2e/app-dir/draft-mode-middleware/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export const metadata = {
title: 'Next.js',
description: 'Generated by Next.js',
}

export default function RootLayout({
children,
}: {
children: React.ReactNode
}) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { draftMode } from 'next/headers'

export default function PreviewPage() {
const { isEnabled } = draftMode()
return <h1>{isEnabled ? 'draft' : 'none'}</h1>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('app-dir - draft-mode-middleware', () => {
const { next, skipped } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})

if (skipped) {
return
}

it('should be able to enable draft mode with middleware present', async () => {
const browser = await next.browser(
'/api/draft?secret=secret-token&slug=preview-page'
)

await retry(async () => {
expect(next.cliOutput).toContain(
'draftMode().isEnabled from middleware: true'
)
})

await browser.loadPage(new URL('/preview-page', next.url).toString())
const draftText = await browser.elementByCss('h1').text()
expect(draftText).toBe('draft')
})

it('should be able to disable draft mode with middleware present', async () => {
const browser = await next.browser('/api/disable-draft')
await retry(async () => {
expect(next.cliOutput).toContain(
'draftMode().isEnabled from middleware: false'
)
})

await browser.loadPage(new URL('/preview-page', next.url).toString())
const draftText = await browser.elementByCss('h1').text()
expect(draftText).toBe('none')
})
})
12 changes: 12 additions & 0 deletions test/e2e/app-dir/draft-mode-middleware/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { NextResponse, type NextRequest } from 'next/server'
import { draftMode } from 'next/headers'

export function middleware(req: NextRequest) {
const { isEnabled } = draftMode()
console.log('draftMode().isEnabled from middleware:', isEnabled)
return NextResponse.next()
}

export const config = {
matcher: ['/((?!_next/static|_next/image|img|assets|ui|favicon.ico).*)'],
}
2 changes: 1 addition & 1 deletion test/e2e/app-dir/draft-mode/draft-mode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('app dir - draft mode', () => {
expect(c).toBeDefined()
})

it('should genenerate rand when draft mode enabled', async () => {
it('should generate rand when draft mode enabled', async () => {
const opts = { headers: { Cookie } }
const $ = await next.render$(basePath, {}, opts)
expect($('#mode').text()).toBe('ENABLED')
Expand Down
4 changes: 2 additions & 2 deletions test/turbopack-dev-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -3412,7 +3412,7 @@
"passed": [
"app dir - draft mode in edge runtime should be disabled from api route handler",
"app dir - draft mode in edge runtime should be enabled from api route handler when draft mode enabled",
"app dir - draft mode in edge runtime should genenerate rand when draft mode enabled",
"app dir - draft mode in edge runtime should generate rand when draft mode enabled",
"app dir - draft mode in edge runtime should have set-cookie header on enable",
"app dir - draft mode in edge runtime should have set-cookie header with redirect location",
"app dir - draft mode in edge runtime should not perform full page navigation on router.refresh()",
Expand All @@ -3421,7 +3421,7 @@
"app dir - draft mode in edge runtime should use initial rand when draft mode is disabled on /with-edge/with-cookies",
"app dir - draft mode in nodejs runtime should be disabled from api route handler",
"app dir - draft mode in nodejs runtime should be enabled from api route handler when draft mode enabled",
"app dir - draft mode in nodejs runtime should genenerate rand when draft mode enabled",
"app dir - draft mode in nodejs runtime should generate rand when draft mode enabled",
"app dir - draft mode in nodejs runtime should have set-cookie header on enable",
"app dir - draft mode in nodejs runtime should have set-cookie header with redirect location",
"app dir - draft mode in nodejs runtime should not perform full page navigation on router.refresh()",
Expand Down

0 comments on commit 8c6608e

Please sign in to comment.