refactor: avoid unnecessary after wrapper (#67463)

I think the wrap function is a leftover from before #66767. We don't
need to return a tuple here, and can just use the `AfterContext`
directly on the call site. This makes its optionality a bit more
obvious, and avoids the noop callback.
This commit is contained in:
Hendrik Liebau 2024-07-05 12:56:53 +02:00 committed by GitHub
parent 3f11815b02
commit 199b869d5b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 35 additions and 46 deletions

View file

@ -4,7 +4,7 @@ import { AsyncLocalStorage } from 'async_hooks'
import type { RequestStore } from '../../client/components/request-async-storage.external'
import type { AfterContext } from './after-context'
describe('createAfterContext', () => {
describe('AfterContext', () => {
// 'async-local-storage.ts' needs `AsyncLocalStorage` on `globalThis` at import time,
// so we have to do some contortions here to set it up before running anything else
type RASMod =
@ -13,7 +13,7 @@ describe('createAfterContext', () => {
type AfterContextMod = typeof import('./after-context')
let requestAsyncStorage: RASMod['requestAsyncStorage']
let createAfterContext: AfterContextMod['createAfterContext']
let AfterContext: AfterContextMod['AfterContext']
let after: AfterMod['unstable_after']
beforeAll(async () => {
@ -26,7 +26,7 @@ describe('createAfterContext', () => {
requestAsyncStorage = RASMod.requestAsyncStorage
const AfterContextMod = await import('./after-context')
createAfterContext = AfterContextMod.createAfterContext
AfterContext = AfterContextMod.AfterContext
const AfterMod = await import('./after')
after = AfterMod.unstable_after
@ -49,7 +49,7 @@ describe('createAfterContext', () => {
onCloseCallback = cb
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -116,7 +116,7 @@ describe('createAfterContext', () => {
onCloseCallback = cb
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -164,7 +164,7 @@ describe('createAfterContext', () => {
onCloseCallback = cb
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -255,7 +255,7 @@ describe('createAfterContext', () => {
onCloseCallback = cb
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -315,7 +315,7 @@ describe('createAfterContext', () => {
throw new Error('onClose is broken for some reason')
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -353,7 +353,7 @@ describe('createAfterContext', () => {
onCloseCallback = cb
})
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -406,7 +406,7 @@ describe('createAfterContext', () => {
const waitUntil = undefined
const onClose = jest.fn()
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,
@ -436,7 +436,7 @@ describe('createAfterContext', () => {
const onClose = undefined
const afterContext = createAfterContext({
const afterContext = new AfterContext({
waitUntil,
onClose,
cacheScope: undefined,

View file

@ -9,22 +9,13 @@ import type { RequestLifecycleOpts } from '../base-server'
import type { AfterCallback, AfterTask } from './after'
import { InvariantError } from '../../shared/lib/invariant-error'
export interface AfterContext {
run<T>(requestStore: RequestStore, callback: () => T): T
after(task: AfterTask): void
}
export type AfterContextOpts = {
waitUntil: RequestLifecycleOpts['waitUntil'] | undefined
onClose: RequestLifecycleOpts['onClose'] | undefined
cacheScope: CacheScope | undefined
}
export function createAfterContext(opts: AfterContextOpts): AfterContext {
return new AfterContextImpl(opts)
}
export class AfterContextImpl implements AfterContext {
export class AfterContext {
private waitUntil: RequestLifecycleOpts['waitUntil'] | undefined
private onClose: RequestLifecycleOpts['onClose'] | undefined
private cacheScope: CacheScope | undefined

View file

@ -20,7 +20,7 @@ import {
import { ResponseCookies, RequestCookies } from '../web/spec-extension/cookies'
import { DraftModeProvider } from './draft-mode-provider'
import { splitCookiesString } from '../web/utils'
import { createAfterContext, type AfterContext } from '../after/after-context'
import { AfterContext } from '../after/after-context'
import type { RequestLifecycleOpts } from '../base-server'
function getHeaders(headers: Headers | IncomingHttpHeaders): ReadonlyHeaders {
@ -84,8 +84,6 @@ export const withRequestStore: WithStore<RequestStore, RequestContext> = <
{ req, url, res, renderOpts }: RequestContext,
callback: (store: RequestStore) => Result
): Result => {
const [wrapWithAfter, afterContext] = createAfterWrapper(renderOpts)
function defaultOnUpdateCookies(cookies: string[]) {
if (res) {
res.setHeader('Set-Cookie', cookies)
@ -172,33 +170,33 @@ export const withRequestStore: WithStore<RequestStore, RequestContext> = <
reactLoadableManifest: renderOpts?.reactLoadableManifest || {},
assetPrefix: renderOpts?.assetPrefix || '',
afterContext,
}
return wrapWithAfter(store, () => storage.run(store, callback, store))
afterContext: createAfterContext(renderOpts),
}
function createAfterWrapper(
if (store.afterContext) {
return store.afterContext.run(store, () =>
storage.run(store, callback, store)
)
}
return storage.run(store, callback, store)
}
function createAfterContext(
renderOpts: WrapperRenderOpts | undefined
): [
wrap: <Result>(requestStore: RequestStore, callback: () => Result) => Result,
afterContext: AfterContext | undefined,
] {
const isAfterEnabled = renderOpts?.experimental?.after ?? false
if (!renderOpts || !isAfterEnabled) {
return [(_, callback) => callback(), undefined]
): AfterContext | undefined {
if (!isAfterEnabled(renderOpts)) {
return undefined
}
const { waitUntil, onClose } = renderOpts
const cacheScope = renderOpts.ComponentMod?.createCacheScope()
const { waitUntil, onClose, ComponentMod } = renderOpts
const cacheScope = ComponentMod?.createCacheScope()
const afterContext = createAfterContext({
waitUntil,
onClose,
cacheScope,
})
const wrap = <Result>(requestStore: RequestStore, callback: () => Result) =>
afterContext.run(requestStore, callback)
return [wrap, afterContext]
return new AfterContext({ waitUntil, onClose, cacheScope })
}
function isAfterEnabled(
renderOpts: WrapperRenderOpts | undefined
): renderOpts is WrapperRenderOpts {
return renderOpts?.experimental?.after ?? false
}