Skip to content

Commit 174fc2f

Browse files
authored
[after] remove createCacheScope (#68744)
This PR removes `createCacheScope` and the associated React monkeypatching 🥳 As discussed, the patch isn't really necessary, and extending the lifetime of `cache()` works by just staying in the same async context (i.e. chaining promises so that AsyncLocalStorage is preserved). I added a note in `after-context` to highlight the place where this happens. Note that, for now, we're still shadowing requestAsyncStorage when running the callbacks (but this may change in the future). What we DIDN'T know is that it **already sometimes worked this way** -- the react patch was partially broken in some scenarios (HMR in dev mode) causing cache scopeto do nothing, but we were already staying in the same async context, and that's how `cache()` still worked. Removing `createCacheScope` also removes the (currently undesireable, because it's inconsistent) side-effect of making `cache()` work in actions. Follow up work will be needed to make sure things like `cookies.set()` are always blocked in `after` (mostly for the `const c = cookies(); after(() => c.set(...))` case) Note that this obsoletes #68547, which was attempting to fix the brokenness of the react patch in HMR.
1 parent 73c8d41 commit 174fc2f

File tree

7 files changed

+53
-210
lines changed

7 files changed

+53
-210
lines changed

packages/next/src/server/after/after-context.test.ts

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ describe('AfterContext', () => {
5252
const afterContext = new AfterContext({
5353
waitUntil,
5454
onClose,
55-
cacheScope: undefined,
5655
})
5756

5857
const requestStore = createMockRequestStore(afterContext)
@@ -119,7 +118,6 @@ describe('AfterContext', () => {
119118
const afterContext = new AfterContext({
120119
waitUntil,
121120
onClose,
122-
cacheScope: undefined,
123121
})
124122

125123
const requestStore = createMockRequestStore(afterContext)
@@ -167,7 +165,6 @@ describe('AfterContext', () => {
167165
const afterContext = new AfterContext({
168166
waitUntil,
169167
onClose,
170-
cacheScope: undefined,
171168
})
172169

173170
const requestStore = createMockRequestStore(afterContext)
@@ -258,7 +255,6 @@ describe('AfterContext', () => {
258255
const afterContext = new AfterContext({
259256
waitUntil,
260257
onClose,
261-
cacheScope: undefined,
262258
})
263259

264260
const requestStore = createMockRequestStore(afterContext)
@@ -318,7 +314,6 @@ describe('AfterContext', () => {
318314
const afterContext = new AfterContext({
319315
waitUntil,
320316
onClose,
321-
cacheScope: undefined,
322317
})
323318

324319
const requestStore = createMockRequestStore(afterContext)
@@ -356,7 +351,6 @@ describe('AfterContext', () => {
356351
const afterContext = new AfterContext({
357352
waitUntil,
358353
onClose,
359-
cacheScope: undefined,
360354
})
361355

362356
const requestStore = createMockRequestStore(afterContext)
@@ -409,7 +403,6 @@ describe('AfterContext', () => {
409403
const afterContext = new AfterContext({
410404
waitUntil,
411405
onClose,
412-
cacheScope: undefined,
413406
})
414407

415408
const requestStore = createMockRequestStore(afterContext)
@@ -439,7 +432,6 @@ describe('AfterContext', () => {
439432
const afterContext = new AfterContext({
440433
waitUntil,
441434
onClose,
442-
cacheScope: undefined,
443435
})
444436

445437
const requestStore = createMockRequestStore(afterContext)
@@ -459,6 +451,47 @@ describe('AfterContext', () => {
459451
expect(waitUntil).not.toHaveBeenCalled()
460452
expect(afterCallback1).not.toHaveBeenCalled()
461453
})
454+
455+
it('shadows requestAsyncStorage within after callbacks', async () => {
456+
const waitUntil = jest.fn()
457+
458+
let onCloseCallback: (() => void) | undefined = undefined
459+
const onClose = jest.fn((cb) => {
460+
onCloseCallback = cb
461+
})
462+
463+
const afterContext = new AfterContext({
464+
waitUntil,
465+
onClose,
466+
})
467+
468+
const requestStore = createMockRequestStore(afterContext)
469+
const run = createRun(afterContext, requestStore)
470+
471+
// ==================================
472+
473+
const stores = new DetachedPromise<
474+
[RequestStore | undefined, RequestStore | undefined]
475+
>()
476+
477+
await run(async () => {
478+
const store1 = requestAsyncStorage.getStore()
479+
after(() => {
480+
const store2 = requestAsyncStorage.getStore()
481+
stores.resolve([store1, store2])
482+
})
483+
})
484+
485+
// the response is done.
486+
onCloseCallback!()
487+
488+
const [store1, store2] = await stores.promise
489+
// if we use .toBe, the proxy from createMockRequestStore throws because jest checks '$$typeof'
490+
expect(store1).toBeTruthy()
491+
expect(store2).toBeTruthy()
492+
expect(store1 === requestStore).toBe(true)
493+
expect(store2 !== store1).toBe(true)
494+
})
462495
})
463496

464497
const createMockRequestStore = (afterContext: AfterContext): RequestStore => {

packages/next/src/server/after/after-context.ts

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
requestAsyncStorage,
44
type RequestStore,
55
} from '../../client/components/request-async-storage.external'
6-
import type { CacheScope } from './react-cache-scope'
76
import { ResponseCookies } from '../web/spec-extension/cookies'
87
import type { RequestLifecycleOpts } from '../base-server'
98
import type { AfterCallback, AfterTask } from './after'
@@ -12,35 +11,28 @@ import { InvariantError } from '../../shared/lib/invariant-error'
1211
export type AfterContextOpts = {
1312
waitUntil: RequestLifecycleOpts['waitUntil'] | undefined
1413
onClose: RequestLifecycleOpts['onClose'] | undefined
15-
cacheScope: CacheScope | undefined
1614
}
1715

1816
export class AfterContext {
1917
private waitUntil: RequestLifecycleOpts['waitUntil'] | undefined
2018
private onClose: RequestLifecycleOpts['onClose'] | undefined
21-
private cacheScope: CacheScope | undefined
2219

2320
private requestStore: RequestStore | undefined
2421

2522
private runCallbacksOnClosePromise: Promise<void> | undefined
2623
private callbackQueue: PromiseQueue
2724

28-
constructor({ waitUntil, onClose, cacheScope }: AfterContextOpts) {
25+
constructor({ waitUntil, onClose }: AfterContextOpts) {
2926
this.waitUntil = waitUntil
3027
this.onClose = onClose
31-
this.cacheScope = cacheScope
3228

3329
this.callbackQueue = new PromiseQueue()
3430
this.callbackQueue.pause()
3531
}
3632

3733
public run<T>(requestStore: RequestStore, callback: () => T): T {
3834
this.requestStore = requestStore
39-
if (this.cacheScope) {
40-
return this.cacheScope.run(() => callback())
41-
} else {
42-
return callback()
43-
}
35+
return callback()
4436
}
4537

4638
public after(task: AfterTask): void {
@@ -78,6 +70,10 @@ export class AfterContext {
7870

7971
// this should only happen once.
8072
if (!this.runCallbacksOnClosePromise) {
73+
// NOTE: We're creating a promise here, which means that
74+
// we will propagate any AsyncLocalStorage contexts we're currently in
75+
// to the callbacks that'll execute later.
76+
// This includes e.g. `requestAsyncStorage` and React's `requestStorage` (which backs `React.cache()`).
8177
this.runCallbacksOnClosePromise = this.runCallbacksOnClose()
8278
this.waitUntil(this.runCallbacksOnClosePromise)
8379
}
@@ -105,20 +101,12 @@ export class AfterContext {
105101
private async runCallbacks(requestStore: RequestStore): Promise<void> {
106102
if (this.callbackQueue.size === 0) return
107103

108-
const runCallbacksImpl = async () => {
109-
this.callbackQueue.start()
110-
return this.callbackQueue.onIdle()
111-
}
112-
113104
const readonlyRequestStore: RequestStore =
114105
wrapRequestStoreForAfterCallbacks(requestStore)
115106

116107
return requestAsyncStorage.run(readonlyRequestStore, () => {
117-
if (this.cacheScope) {
118-
return this.cacheScope.run(runCallbacksImpl)
119-
} else {
120-
return runCallbacksImpl()
121-
}
108+
this.callbackQueue.start()
109+
return this.callbackQueue.onIdle()
122110
})
123111
}
124112
}

packages/next/src/server/after/react-cache-scope.ts

Lines changed: 0 additions & 160 deletions
This file was deleted.

packages/next/src/server/app-render/app-render.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -854,10 +854,6 @@ async function renderToHTMLOrFlightImpl(
854854

855855
ComponentMod.patchFetch()
856856

857-
if (renderOpts.experimental.after) {
858-
ComponentMod.patchCacheScopeSupportIntoReact()
859-
}
860-
861857
// Pull out the hooks/references from the component.
862858
const { tree: loaderTree, taintObjectReference } = ComponentMod
863859

packages/next/src/server/app-render/entry-base.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,6 @@ import {
3030
import { Postpone } from '../../server/app-render/rsc/postpone'
3131
import { taintObjectReference } from '../../server/app-render/rsc/taint'
3232

33-
import * as React from 'react'
34-
import {
35-
patchCacheScopeSupportIntoReact as _patchCacheScopeSupportIntoReact,
36-
createCacheScope,
37-
} from '../after/react-cache-scope'
38-
39-
function patchCacheScopeSupportIntoReact() {
40-
_patchCacheScopeSupportIntoReact(React)
41-
}
42-
4333
// patchFetch makes use of APIs such as `React.unstable_postpone` which are only available
4434
// in the experimental channel of React, so export it from here so that it comes from the bundled runtime
4535
function patchFetch() {
@@ -66,6 +56,4 @@ export {
6656
ClientPageRoot,
6757
NotFoundBoundary,
6858
patchFetch,
69-
createCacheScope,
70-
patchCacheScopeSupportIntoReact,
7159
}

0 commit comments

Comments
 (0)