Skip to content

Commit

Permalink
[dynamicIO] Instrument Math.random() to be considered synchronously…
Browse files Browse the repository at this point in the history
… dynamic (#70837)

`Math.random()` is IO though it is synchronous. When `dynamicIO` is
enabled we need to treat calls to `Math.random()` as IO and exclude them
from prerenders unless they are cached. This change sets up the
instrumentation for this behavior.

Unless the sync access of async request APIs this patch does not yet
have any dev warnings. These will be added in a future update when we
can instrument the environment name during dev to tell the difference
between the prerender phase, and render phase even without a build
  • Loading branch information
gnoff authored Oct 7, 2024
1 parent 1e000f6 commit 9e0ce4f
Show file tree
Hide file tree
Showing 70 changed files with 1,637 additions and 35 deletions.
19 changes: 11 additions & 8 deletions packages/next/src/server/app-render/dynamic-rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,18 @@ export function trackDynamicDataInDynamicRender(
// Despite it's name we don't actually abort unless we have a controller to call abort on
// There are times when we let a prerender run long to discover caches where we want the semantics
// of tracking dynamic access without terminating the prerender early
function abortOnSynchronousDynamicDataAccess(
export function abortOnSynchronousDynamicDataAccess(
route: string,
expression: string,
prerenderStore: PrerenderStoreModern
): void {
if (prerenderStore.dynamicTracking) {
const disallowedDynamic = prerenderStore.dynamicTracking.disallowedDynamic
if (disallowedDynamic.syncDynamicExpression === '') {
disallowedDynamic.syncDynamicExpression = expression
}
}

const reason = `Route ${route} needs to bail out of prerendering at this point because it used ${expression}.`

const error = createPrerenderInterruptedError(reason)
Expand Down Expand Up @@ -367,12 +374,6 @@ export function abortAndThrowOnSynchronousDynamicDataAccess(
expression: string,
prerenderStore: PrerenderStoreModern
): never {
if (prerenderStore.dynamicTracking) {
const disallowedDynamic = prerenderStore.dynamicTracking.disallowedDynamic
if (disallowedDynamic.syncDynamicExpression === '') {
disallowedDynamic.syncDynamicExpression = expression
}
}
abortOnSynchronousDynamicDataAccess(route, expression, prerenderStore)
throw createPrerenderInterruptedError(
`Route ${route} needs to bail out of prerendering at this point because it used ${expression}.`
Expand Down Expand Up @@ -674,8 +675,10 @@ export function throwIfDisallowedDynamic(
for (let i = 0; i < syncDynamicErrors.length; i++) {
console.error(syncDynamicErrors[i])
}
const expression =
disallowedDynamic.syncDynamicExpression || 'a synchronous Dynamic API'
throw new StaticGenBailoutError(
`Route ${workStore.route} used a synchronous Dynamic API while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI. It is best to avoid synchronous Dynamic API access during prerendering.`
`Route ${workStore.route} used ${expression} while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI. It is best to avoid synchronous Dynamic API access during prerendering.`
)
}

Expand Down
16 changes: 16 additions & 0 deletions packages/next/src/server/node-environment-baseline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// This file should be imported before any others. It sets up the environment
// for later imports to work properly.

// expose AsyncLocalStorage on global for react usage if it isn't already provided by the environment
if (typeof (globalThis as any).AsyncLocalStorage !== 'function') {
const { AsyncLocalStorage } = require('async_hooks')
;(globalThis as any).AsyncLocalStorage = AsyncLocalStorage
}

if (typeof (globalThis as any).WebSocket !== 'function') {
Object.defineProperty(globalThis, 'WebSocket', {
get() {
return require('next/dist/compiled/ws').WebSocket
},
})
}
39 changes: 39 additions & 0 deletions packages/next/src/server/node-environment-extensions/random.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* We extend Math.random() during builds and revalidates to ensure that prerenders don't observe randomness
* When dynamicIO is enabled. randomness is a form of IO even though it resolves synchronously. When dyanmicIO is
* enabled we need to ensure that randomness is excluded from prerenders.
*
* The extensions here never error nor alter the random generation itself and thus should be transparent to callers.
*/

import { workAsyncStorage } from '../../client/components/work-async-storage.external'
import {
isDynamicIOPrerender,
prerenderAsyncStorage,
} from '../app-render/prerender-async-storage.external'
import { abortOnSynchronousDynamicDataAccess } from '../app-render/dynamic-rendering'

const originalRandom = Math.random

Math.random = function random() {
const prerenderStore = prerenderAsyncStorage.getStore()
if (
prerenderStore &&
prerenderStore.type === 'prerender' &&
isDynamicIOPrerender(prerenderStore)
) {
const workStore = workAsyncStorage.getStore()
const route = workStore ? workStore.route : ''
const expression = '`Math.random()`'
if (workStore) {
abortOnSynchronousDynamicDataAccess(route, expression, prerenderStore)
}
}

return originalRandom.apply(null, arguments as any)

// We bind here to alter the `toString` printing to match `Math.random`'s native `toString`.
// eslint-disable-next-line no-extra-bind
}.bind(null)

Object.defineProperty(Math.random, 'name', { value: 'random' })
15 changes: 2 additions & 13 deletions packages/next/src/server/node-environment.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
// This file should be imported before any others. It sets up the environment
// for later imports to work properly.

// expose AsyncLocalStorage on global for react usage if it isn't already provided by the environment
if (typeof (globalThis as any).AsyncLocalStorage !== 'function') {
const { AsyncLocalStorage } = require('async_hooks')
;(globalThis as any).AsyncLocalStorage = AsyncLocalStorage
}

if (typeof (globalThis as any).WebSocket !== 'function') {
Object.defineProperty(globalThis, 'WebSocket', {
get() {
return require('next/dist/compiled/ws').WebSocket
},
})
}
import './node-environment-baseline'
import './node-environment-extensions/random'
9 changes: 9 additions & 0 deletions packages/next/src/server/route-modules/app-route/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,15 @@ export class AppRouteRouteModule extends RouteModule<
}
})
})
if (controller.signal.aborted) {
// We aborted from within the execution
throw createDynamicIOError(workStore.route)
} else {
// We didn't abort during the execution. We can abort now as a matter of semantics
// though at the moment nothing actually consumes this signal so it won't halt any
// inflight work.
controller.abort()
}
} else if (isStaticGeneration) {
res = await workUnitAsyncStorage.run(
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
import { nextTestSetup } from 'e2e-utils'

const WITH_PPR = !!process.env.__NEXT_EXPERIMENTAL_PPR

const stackStart = /\s+at /

function createExpectError(cliOutput: string) {
let cliIndex = 0
return function expectError(
containing: string,
withStackContaining?: string
) {
const initialCliIndex = cliIndex
let lines = cliOutput.slice(cliIndex).split('\n')

let i = 0
while (i < lines.length) {
let line = lines[i++] + '\n'
cliIndex += line.length
if (line.includes(containing)) {
if (typeof withStackContaining !== 'string') {
return
} else {
while (i < lines.length) {
let stackLine = lines[i++] + '\n'
if (!stackStart.test(stackLine)) {
expect(stackLine).toContain(withStackContaining)
}
if (stackLine.includes(withStackContaining)) {
return
}
}
}
}
}

expect(cliOutput.slice(initialCliIndex)).toContain(containing)
}
}

function runTests(options: { withMinification: boolean }) {
const isTurbopack = !!process.env.TURBOPACK
const { withMinification } = options
describe(`Dynamic IO Errors - ${withMinification ? 'With Minification' : 'Without Minification'}`, () => {
describe('Sync Dynamic - With Fallback - Math.random()', () => {
const { next, isNextDev, skipped } = nextTestSetup({
files: __dirname + '/fixtures/sync-random-with-fallback',
skipStart: true,
})

if (skipped) {
return
}

if (isNextDev) {
it('does not run in dev', () => {})
return
}

beforeEach(async () => {
if (!withMinification) {
await next.patchFile('next.config.js', (content) =>
content.replace(
'serverMinification: true,',
'serverMinification: false,'
)
)
}
})

it('should not error the build when calling Math.random() if all dynamic access is inside a Suspense boundary', async () => {
try {
await next.start()
} catch {
throw new Error('expected build not to fail for fully static project')
}

if (WITH_PPR) {
expect(next.cliOutput).toContain('◐ / ')
const $ = await next.render$('/')
expect($('[data-fallback]').length).toBe(2)
} else {
expect(next.cliOutput).toContain('ƒ / ')
const $ = await next.render$('/')
expect($('[data-fallback]').length).toBe(0)
}
})
})

describe('Sync Dynamic - Without Fallback - Math.random()', () => {
const { next, isNextDev, skipped } = nextTestSetup({
files: __dirname + '/fixtures/sync-random-without-fallback',
skipStart: true,
})

if (skipped) {
return
}

if (isNextDev) {
it('does not run in dev', () => {})
return
}

beforeEach(async () => {
if (!withMinification) {
await next.patchFile('next.config.js', (content) =>
content.replace(
'serverMinification: true,',
'serverMinification: false,'
)
)
}
})

it('should error the build if Math.random() happens before some component outside a Suspense boundary is complete', async () => {
try {
await next.start()
} catch {
// we expect the build to fail
}
const expectError = createExpectError(next.cliOutput)

if (WITH_PPR) {
expectError(
'Error: Route / used a synchronous Dynamic API: `Math.random()`. This particular component may have been dynamic anyway or it may have just not finished before the synchronous Dynamic API was invoked.',
// Turbopack doesn't support disabling minification yet
withMinification || isTurbopack ? undefined : 'IndirectionTwo'
)
} else {
expectError(
'Error: Route / used a synchronous Dynamic API: `Math.random()`, which caused this component to not finish rendering before the prerender completed and no fallback UI was defined.',
// Turbopack doesn't support disabling minification yet
withMinification || isTurbopack ? undefined : 'IndirectionTwo'
)
}
expectError('Error occurred prerendering page "/"')
expectError(
'Error: Route / used `Math.random()` while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
)
expectError('exiting the build.')
})
})
})
}

runTests({ withMinification: true })
runTests({ withMinification: false })
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function runTests(options: { withMinification: boolean }) {
)
expectError('Error occurred prerendering page "/"')
expectError(
'Error: Route / used a synchronous Dynamic API while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
'Error: Route / used `searchParams.foo` while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
)
expectError('exiting the build.')
})
Expand Down Expand Up @@ -228,7 +228,7 @@ function runTests(options: { withMinification: boolean }) {
}
expectError('Error occurred prerendering page "/"')
expectError(
'Error: Route / used a synchronous Dynamic API while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
'Error: Route / used `searchParams.foo` while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
)
expectError('exiting the build.')
})
Expand Down Expand Up @@ -328,7 +328,7 @@ function runTests(options: { withMinification: boolean }) {
}
expectError('Error occurred prerendering page "/"')
expectError(
'Error: Route / used a synchronous Dynamic API while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI.'
"Error: Route / used cookies().get('token') while prerendering which caused some part of the page to be dynamic without a Suspense boundary above it defining a fallback UI."
)
expectError('exiting the build.')
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use client'

export function IndirectionOne({ children }) {
return children
}

export function IndirectionTwo({ children }) {
return children
}

export function IndirectionThree({ children }) {
return children
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<body>
<main>{children}</main>
</body>
</html>
)
}
Loading

0 comments on commit 9e0ce4f

Please sign in to comment.