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

Reapply "Dedupe sync access warning on the Server by callsite" (#70672) #70738

Merged
merged 3 commits into from
Oct 3, 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
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
Loading