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

Fix edge preview props are not matched with cookie #67779

Merged
merged 8 commits into from
Jul 15, 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
10 changes: 4 additions & 6 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values were the main reason this manifest was added iirc so should we just fully remove it?

Could be in a separate PR just noting for cleanup purposes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I'll get a follow up PR to remove it from edge runtime

// 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 @@ -36,6 +36,7 @@ import { UNDERSCORE_NOT_FOUND_ROUTE } from '../api/constants'
import type { DeepReadonly } from '../shared/lib/deep-readonly'
import { getEdgeInstrumentationModule } from './web/globals'
import type { ServerOnInstrumentationRequestError } from './app-render/types'
import { getEdgePreviewProps } from './web/get-edge-preview-props'

interface WebServerOptions extends Options {
webServerConfig: {
Expand Down Expand Up @@ -146,9 +147,7 @@ export default class NextWebServer extends BaseServer<
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, normalizeNextQueryParam } from './utils'
import { NextFetchEvent } from './spec-extension/fetch-event'
Expand All @@ -22,6 +21,7 @@ import { getTracer } from '../lib/trace/tracer'
import type { TextMapGetter } from 'next/dist/compiled/@opentelemetry/api'
import { MiddlewareSpan } from '../lib/trace/constants'
import { CloseController } from './web-on-close'
import { getEdgePreviewProps } from './get-edge-preview-props'

export class NextRequestHint extends NextRequest {
sourcePage: string
Expand Down Expand Up @@ -93,10 +93,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 @@ -196,9 +192,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 @@ -240,11 +234,7 @@ export async function adapter(
},
async () => {
try {
const previewProps = prerenderManifest?.preview || {
previewModeId: 'development-id',
previewModeEncryptionKey: '',
previewModeSigningKey: '',
}
const previewProps = getEdgePreviewProps()

return await withRequestStore(
requestAsyncStorage,
Expand Down
15 changes: 4 additions & 11 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 '../route-modules/app-route/module'
import type { PrerenderManifest } from '../../build'

import './globals'

Expand All @@ -16,6 +15,7 @@ import { getUtils } from '../server-utils'
import { searchParamsToUrlQuery } from '../../shared/lib/router/utils/querystring'
import type { RequestLifecycleOpts } from '../base-server'
import { CloseController, trackStreamConsumed } from './web-on-close'
import { getEdgePreviewProps } from './get-edge-preview-props'

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

Expand Down Expand Up @@ -84,11 +84,6 @@ export class EdgeRouteModuleWrapper {
searchParamsToUrlQuery(request.nextUrl.searchParams)
)

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

const isAfterEnabled = !!process.env.__NEXT_AFTER

let waitUntil: RequestLifecycleOpts['waitUntil'] = undefined
Expand All @@ -99,6 +94,8 @@ export class EdgeRouteModuleWrapper {
closeController = new CloseController()
}

const previewProps = getEdgePreviewProps()

// Create the context for the handler. This contains the params from the
// match (if any).
const context: AppRouteRouteHandlerContext = {
Expand All @@ -107,11 +104,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 @@ -66,7 +66,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 @@ -3835,7 +3835,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 @@ -3844,7 +3844,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
Loading