Skip to content

Commit

Permalink
Reapply "Dedupe sync access warning on the Server by callsite" (#70672)…
Browse files Browse the repository at this point in the history
… (#70738)

Relands #70672 with fixes that
were failing in CI.


[x-ref](https://github.com/vercel/next.js/actions/runs/11153352206/job/31002090099)
  • Loading branch information
ztanner authored Oct 3, 2024
1 parent b68c188 commit a4763cc
Show file tree
Hide file tree
Showing 17 changed files with 484 additions and 103 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import * as React from 'react'

const errorRef: { current: null | string } = { current: null }

// React.cache is currently only available in canary/experimental React channels.
const cache =
typeof React.cache === 'function'
? React.cache
: (fn: (key: unknown) => void) => fn

// We don't want to dedupe across requests.
// The developer might've just attempted to fix the warning so we should warn again if it still happens.
const flushCurrentErrorIfNew = cache(
// eslint-disable-next-line @typescript-eslint/no-unused-vars -- cache key
(key: unknown) => {
try {
console.error(errorRef.current)
} finally {
errorRef.current = null
}
}
)

/**
* Creates a function that logs an error message that is deduped by the userland
* callsite.
* This requires no indirection between the call of this function and the userland
* callsite i.e. there's only a single library frame above this.
* Do not use on the Client where sourcemaps and ignore listing might be enabled.
* Only use that for warnings need a fix independent of the callstack.
*
* @param getMessage
* @returns
*/
export function createDedupedByCallsiteServerErrorLoggerDev<Args extends any[]>(
getMessage: (...args: Args) => string
) {
return function logDedupedError(...args: Args) {
const message = getMessage(...args)

if (process.env.NODE_ENV !== 'production') {
const callStackFrames = new Error().stack?.split('\n')
if (callStackFrames === undefined || callStackFrames.length < 4) {
console.error(message)
} else {
// Error:
// logDedupedError
// asyncApiBeingAccessedSynchronously
// <userland callsite>
// TODO: This breaks if sourcemaps with ignore lists are enabled.
const key = callStackFrames[3]
errorRef.current = message
flushCurrentErrorIfNew(key)
}
} else {
console.error(message)
}
}
}
28 changes: 17 additions & 11 deletions packages/next/src/server/request/cookies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { actionAsyncStorage } from '../../client/components/action-async-storage
import { StaticGenBailoutError } from '../../client/components/static-generation-bailout'
import { makeResolvedReactPromise } from './utils'
import { makeHangingPromise } from '../dynamic-rendering-utils'
import { createDedupedByCallsiteServerErrorLoggerDev } from '../create-deduped-by-callsite-server-error-loger'

/**
* In this version of Next.js `cookies()` returns a Promise however you can still reference the properties of the underlying cookies object
Expand Down Expand Up @@ -521,25 +522,30 @@ const noop = () => {}
const warnForSyncIteration = process.env
.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncIteration(route?: string) {
const prefix = route ? ` In route ${route} ` : ''
console.error(
`${prefix}cookies were iterated over. ` +
: createDedupedByCallsiteServerErrorLoggerDev(
function getSyncIterationMessage(route?: string) {
const prefix = route ? ` In route ${route} ` : ''
return (
`${prefix}cookies were iterated over. ` +
`\`cookies()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
)
}
)

const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncAccess(route: undefined | string, expression: string) {
: createDedupedByCallsiteServerErrorLoggerDev(function getSyncAccessMessage(
route: undefined | string,
expression: string
) {
const prefix = route ? ` In route ${route} a ` : 'A '
console.error(
return (
`${prefix}cookie property was accessed directly with \`${expression}\`. ` +
`\`cookies()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`cookies()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
})

function polyfilledResponseCookiesIterator(
this: ResponseCookies
Expand Down
14 changes: 9 additions & 5 deletions packages/next/src/server/request/draft-mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { DraftModeProvider } from '../../server/async-storage/draft-mode-pr
import { workAsyncStorage } from '../../client/components/work-async-storage.external'
import { cacheAsyncStorage } from '../../server/app-render/cache-async-storage.external'
import { trackDynamicDataAccessed } from '../app-render/dynamic-rendering'
import { createDedupedByCallsiteServerErrorLoggerDev } from '../create-deduped-by-callsite-server-error-loger'

/**
* In this version of Next.js `draftMode()` returns a Promise however you can still reference the properties of the underlying draftMode object
Expand Down Expand Up @@ -167,11 +168,14 @@ const noop = () => {}

const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncAccess(route: undefined | string, expression: string) {
: createDedupedByCallsiteServerErrorLoggerDev(function getSyncAccessWarning(
route: undefined | string,
expression: string
) {
const prefix = route ? ` In route ${route} a ` : 'A '
console.error(
return (
`${prefix}\`draftMode()\` property was accessed directly with \`${expression}\`. ` +
`\`draftMode()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/draft-mode-sync-access`
`\`draftMode()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/draft-mode-sync-access`
)
}
})
28 changes: 17 additions & 11 deletions packages/next/src/server/request/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { StaticGenBailoutError } from '../../client/components/static-generation-bailout'
import { makeResolvedReactPromise } from './utils'
import { makeHangingPromise } from '../dynamic-rendering-utils'
import { createDedupedByCallsiteServerErrorLoggerDev } from '../create-deduped-by-callsite-server-error-loger'

/**
* In this version of Next.js `headers()` returns a Promise however you can still reference the properties of the underlying Headers instance
Expand Down Expand Up @@ -436,25 +437,30 @@ const noop = () => {}
const warnForSyncIteration = process.env
.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncIteration(route?: string) {
const prefix = route ? ` In route ${route} ` : ''
console.error(
`${prefix}headers were iterated over. ` +
: createDedupedByCallsiteServerErrorLoggerDev(
function getSyncIterationMessage(route?: string) {
const prefix = route ? ` In route ${route} ` : ''
return (
`${prefix}headers were iterated over. ` +
`\`headers()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
)
}
)

const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncAccess(route: undefined | string, expression: string) {
: createDedupedByCallsiteServerErrorLoggerDev(function getSyncAccessMessage(
route: undefined | string,
expression: string
) {
const prefix = route ? ` In route ${route} a ` : 'A '
console.error(
return (
`${prefix}header property was accessed directly with \`${expression}\`. ` +
`\`headers()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`headers()\` should be awaited before using its value. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
})

type HeadersExtensions = {
[K in keyof ReadonlyHeaders]: unknown
Expand Down
38 changes: 17 additions & 21 deletions packages/next/src/server/request/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { InvariantError } from '../../shared/lib/invariant-error'
import { makeResolvedReactPromise, describeStringPropertyAccess } from './utils'
import { makeHangingPromise } from '../dynamic-rendering-utils'
import { createDedupedByCallsiteServerErrorLoggerDev } from '../create-deduped-by-callsite-server-error-loger'

export type Params = Record<string, string | Array<string> | undefined>

Expand Down Expand Up @@ -491,46 +492,41 @@ const noop = () => {}

const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncAccess(route: undefined | string, expression: string) {
if (process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS) {
return
}

: createDedupedByCallsiteServerErrorLoggerDev(function getSyncAccessMessage(
route: undefined | string,
expression: string
) {
const prefix = route ? ` In route ${route} a ` : 'A '
console.error(
return (
`${prefix}param property was accessed directly with ${expression}. ` +
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
})

const warnForEnumeration = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForEnumeration(
: createDedupedByCallsiteServerErrorLoggerDev(function getEnumerationMessage(
route: undefined | string,
missingProperties: Array<string>
) {
if (process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS) {
return
}

const prefix = route ? ` In route ${route} ` : ''
if (missingProperties.length) {
const describedMissingProperties =
describeListOfPropertyNames(missingProperties)
console.error(
return (
`${prefix}params are being enumerated incompletely missing these properties: ${describedMissingProperties}. ` +
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
} else {
console.error(
return (
`${prefix}params are being enumerated. ` +
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`params\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
}
})

function describeListOfPropertyNames(properties: Array<string>) {
switch (properties.length) {
Expand Down
30 changes: 17 additions & 13 deletions packages/next/src/server/request/search-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { cacheAsyncStorage } from '../app-render/cache-async-storage.external'
import { InvariantError } from '../../shared/lib/invariant-error'
import { makeHangingPromise } from '../dynamic-rendering-utils'
import { createDedupedByCallsiteServerErrorLoggerDev } from '../create-deduped-by-callsite-server-error-loger'
import {
describeStringPropertyAccess,
describeHasCheckingStringProperty,
Expand Down Expand Up @@ -660,38 +661,41 @@ const noop = () => {}

const warnForSyncAccess = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForSyncAccess(route: undefined | string, expression: string) {
: createDedupedByCallsiteServerErrorLoggerDev(function getSyncAccessMessage(
route: undefined | string,
expression: string
) {
const prefix = route ? ` In route ${route} a ` : 'A '
console.error(
return (
`${prefix}searchParam property was accessed directly with ${expression}. ` +
`\`searchParams\` should be awaited before accessing properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`searchParams\` should be awaited before accessing properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
})

const warnForEnumeration = process.env.__NEXT_DISABLE_SYNC_DYNAMIC_API_WARNINGS
? noop
: function warnForEnumeration(
: createDedupedByCallsiteServerErrorLoggerDev(function getEnumerationMessage(
route: undefined | string,
missingProperties: Array<string>
) {
const prefix = route ? ` In route ${route} ` : ''
if (missingProperties.length) {
const describedMissingProperties =
describeListOfPropertyNames(missingProperties)
console.error(
return (
`${prefix}searchParams are being enumerated incompletely missing these properties: ${describedMissingProperties}. ` +
`\`searchParams\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`searchParams\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
} else {
console.error(
return (
`${prefix}searchParams are being enumerated. ` +
`\`searchParams\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
`\`searchParams\` should be awaited before accessing its properties. ` +
`Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis`
)
}
}
})

function describeListOfPropertyNames(properties: Array<string>) {
switch (properties.length) {
Expand Down
1 change: 1 addition & 0 deletions test/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

e2e/**/tsconfig.json
production/**/tsconfig.json
development/**/tsconfig.json

test-junit-report/
turbopack-test-junit-report/
7 changes: 7 additions & 0 deletions test/development/app-dir/dynamic-io-warnings/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { cookies, type UnsafeUnwrappedCookies } from 'next/headers'

function Component() {
;(cookies() as unknown as UnsafeUnwrappedCookies).get('component')
;(cookies() as unknown as UnsafeUnwrappedCookies).has('component')

const allCookies = [...(cookies() as unknown as UnsafeUnwrappedCookies)]
return <pre>{JSON.stringify(allCookies, null, 2)}</pre>
}

export default function Page() {
;(cookies() as unknown as UnsafeUnwrappedCookies).get('page')
return (
<>
<Component />
<Component />
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { draftMode, type UnsafeUnwrappedDraftMode } from 'next/headers'

function Component() {
const isEnabled = (draftMode() as unknown as UnsafeUnwrappedDraftMode)
.isEnabled
;(draftMode() as unknown as UnsafeUnwrappedDraftMode).enable()

const clonedDraftMode = {
...(draftMode() as unknown as UnsafeUnwrappedDraftMode),
}
return <pre>{JSON.stringify({ clonedDraftMode, isEnabled }, null, 2)}</pre>
}

export default function Page() {
const isEnabled = (draftMode() as unknown as UnsafeUnwrappedDraftMode)
.isEnabled
return (
<>
<pre>{JSON.stringify({ isEnabled }, null, 2)}</pre>
<Component />
<Component />
</>
)
}
Loading

0 comments on commit a4763cc

Please sign in to comment.