-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Move AfterContext to WorkStore #70806
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,11 +33,6 @@ export class AfterContext { | |
this.callbackQueue.pause() | ||
} | ||
|
||
public run<T>(requestStore: RequestStore, callback: () => T): T { | ||
this.requestStore = requestStore | ||
return callback() | ||
} | ||
|
||
public after(task: AfterTask): void { | ||
if (isThenable(task)) { | ||
task.catch(() => {}) // avoid unhandled rejection crashes | ||
|
@@ -61,9 +56,10 @@ export class AfterContext { | |
errorWaitUntilNotAvailable() | ||
} | ||
if (!this.requestStore) { | ||
throw new InvariantError( | ||
'unstable_after: Expected `AfterContext.requestStore` to be initialized' | ||
) | ||
// We just stash the first request store we have but this is not sufficient. | ||
// TODO: We should store a request store per callback since each callback might | ||
// be inside a different store. E.g. inside different batched actions, prerenders or caches. | ||
this.requestStore = requestAsyncStorage.getStore() | ||
} | ||
if (!this.onClose) { | ||
throw new InvariantError( | ||
|
@@ -98,19 +94,27 @@ export class AfterContext { | |
|
||
private async runCallbacksOnClose() { | ||
await new Promise<void>((resolve) => this.onClose!(resolve)) | ||
return this.runCallbacks(this.requestStore!) | ||
return this.runCallbacks(this.requestStore) | ||
} | ||
|
||
private async runCallbacks(requestStore: RequestStore): Promise<void> { | ||
private async runCallbacks( | ||
requestStore: undefined | RequestStore | ||
): Promise<void> { | ||
if (this.callbackQueue.size === 0) return | ||
|
||
const readonlyRequestStore: RequestStore = | ||
wrapRequestStoreForAfterCallbacks(requestStore) | ||
const readonlyRequestStore: undefined | RequestStore = | ||
requestStore === undefined | ||
? undefined | ||
: // TODO: This is not sufficient. It should just be the same store that mutates. | ||
wrapRequestStoreForAfterCallbacks(requestStore) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lubieowoce Instead of wrapping, we should mutate something in the request store when we change scope to handle the case where it's started earlier. |
||
|
||
const workStore = workAsyncStorage.getStore() | ||
|
||
return withExecuteRevalidates(workStore, () => | ||
requestAsyncStorage.run(readonlyRequestStore, async () => { | ||
// Clearing it out or running the first request store. | ||
// TODO: This needs to be the request store that was active at the time the | ||
// callback was scheduled but p-queue makes this hard so need further refactoring. | ||
requestAsyncStorage.run(readonlyRequestStore as any, async () => { | ||
this.callbackQueue.start() | ||
await this.callbackQueue.onIdle() | ||
}) | ||
|
@@ -141,7 +145,6 @@ function wrapRequestStoreForAfterCallbacks( | |
}, | ||
// TODO(after): calling a `cookies.set()` in an after() that's in an action doesn't currently error. | ||
mutableCookies: new ResponseCookies(new Headers()), | ||
afterContext: requestStore.afterContext, | ||
isHmrRefresh: requestStore.isHmrRefresh, | ||
serverComponentsHmrCache: requestStore.serverComponentsHmrCache, | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
import { requestAsyncStorage } from '../../client/components/request-async-storage.external' | ||
import { workAsyncStorage } from '../../client/components/work-async-storage.external' | ||
import { cacheAsyncStorage } from '../../server/app-render/cache-async-storage.external' | ||
import { StaticGenBailoutError } from '../../client/components/static-generation-bailout' | ||
|
@@ -11,36 +10,30 @@ export type AfterCallback<T = unknown> = () => T | Promise<T> | |
/** | ||
* This function allows you to schedule callbacks to be executed after the current request finishes. | ||
*/ | ||
export function unstable_after<T>(task: AfterTask<T>) { | ||
const callingExpression = 'unstable_after' | ||
|
||
// TODO: This is not safe. afterContext should move to WorkStore. | ||
const requestStore = requestAsyncStorage.getStore() | ||
if (!requestStore) { | ||
throw new Error( | ||
`\`${callingExpression}\` was called outside a request scope. Read more: https://nextjs.org/docs/messages/next-dynamic-api-wrong-context` | ||
) | ||
} | ||
|
||
const { afterContext } = requestStore | ||
if (!afterContext) { | ||
throw new Error( | ||
'`unstable_after()` must be explicitly enabled by setting `experimental.after: true` in your next.config.js.' | ||
) | ||
} | ||
|
||
export function unstable_after<T>(task: AfterTask<T>): void { | ||
const workStore = workAsyncStorage.getStore() | ||
const cacheStore = cacheAsyncStorage.getStore() | ||
|
||
if (workStore) { | ||
const { afterContext } = workStore | ||
if (!afterContext) { | ||
throw new Error( | ||
'`unstable_after()` must be explicitly enabled by setting `experimental.after: true` in your next.config.js.' | ||
) | ||
} | ||
|
||
// TODO: After should not cause dynamic. | ||
const callingExpression = 'unstable_after' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lubieowoce As discussed. This should be removed and we should now only run it during the prerender. |
||
if (workStore.forceStatic) { | ||
throw new StaticGenBailoutError( | ||
`Route ${workStore.route} with \`dynamic = "force-static"\` couldn't be rendered statically because it used \`${callingExpression}\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering` | ||
) | ||
} else { | ||
markCurrentScopeAsDynamic(workStore, cacheStore, callingExpression) | ||
} | ||
} | ||
|
||
return afterContext.after(task) | ||
afterContext.after(task) | ||
} else { | ||
// TODO: Error for pages? | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lubieowoce #70810