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
  • Loading branch information
gnoff committed Oct 7, 2024
1 parent a5d2190 commit 58b1009
Show file tree
Hide file tree
Showing 70 changed files with 1,710 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 @@ -317,11 +317,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 @@ -359,12 +366,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 @@ -666,8 +667,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
},
})
}
36 changes: 36 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,36 @@
/**
* 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 () {
const workStore = workAsyncStorage.getStore()
if (workStore) {
const prerenderStore = prerenderAsyncStorage.getStore()
if (
prerenderStore &&
prerenderStore.type === 'prerender' &&
isDynamicIOPrerender(prerenderStore)
) {
abortOnSynchronousDynamicDataAccess(
workStore.route,
'`Math.random()`',
prerenderStore
)
}
}

return originalRandom.apply(this, arguments as any)
}
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 prerenderAsyncStorage.run(
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
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)

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 58b1009

Please sign in to comment.