gsadmin/gitbook
1
0
Fork 0
mirror of https://github.com/GitbookIO/gitbook.git synced 2026-09-11 21:39:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "super hacky solution to PPR cache body"

Browse Source 
This reverts commit 4c36cf9abd.
This commit is contained in:
Nicolas Dorseuil
2026-09-07 11:43:52 +02:00
parent 4c36cf9abd
commit 3bd9976524
11 changed files with 15 additions and 451 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/gitbook/src/app/utils.test.ts
+9 -98
View File
@@ -1,58 +1,18 @@
import { afterAll, beforeAll, describe, expect, it, mock } from 'bun:test';
import jwt from 'jsonwebtoken';
import {
type WorkStore,
workAsyncStorage,
} from 'next/dist/server/app-render/work-async-storage.external';
import rison from 'rison';
import * as realContext from '@/lib/context';
mock.module('server-only', () => ({}));
// Only the lookups are stubbed: mocking the whole module would leak into the other test files,
// Only the lookup is stubbed: mocking the whole module would leak into the other test files,
// as `mock.module` replaces it for the entire test process.
mock.module('@/lib/context', () => ({
...realContext,
getBaseContext: (input: BaseContextInput) => ({
...input,
dataFetcher: createFakeDataFetcher(input),
}),
getBaseContext: (input: unknown) => input,
fetchSiteContextByURLLookup: async (_baseContext: unknown, data: unknown) => data,
// Just enough of a site scope for the real merge with a space context.
fetchSiteScopeContextByURLLookup: async (
_baseContext: unknown,
{ revision, ...data }: { siteSpace: string | undefined; revision: string }
) => ({
...data,
revisionId: revision,
siteSpace: { id: data.siteSpace, space: {} },
linker: 'site-scope-linker',
}),
fetchSiteScopeContextByURLLookup: async (_baseContext: unknown, data: unknown) => data,
}));
type BaseContextInput = { siteURLData: { apiToken: string }; pprScope?: string };
/** Records the token and scope it was created with, so the composed context can be asserted on. */
function createFakeDataFetcher(input: BaseContextInput) {
const fetchedWith = {
pprScope: input.pprScope,
tokenScope: getTokenScope(input.siteURLData.apiToken),
};
return {
fetchedWith,
getSpace: async (params: { spaceId: string }) => ({
data: { id: params.spaceId, organization: 'org-id', revision: 'space-revision-id' },
}),
getRevision: async (params: { revisionId: string }) => ({
data: { id: params.revisionId, pages: [], fetchedWith },
}),
};
}
function getTokenScope(token: string): string | undefined {
return (jwt.decode(token) as { claims?: { scope?: string } } | null)?.claims?.scope;
}
// Stand in for the exchange endpoint, which is the only thing that can narrow the claims. It is
// stubbed at the network boundary rather than with `mock.module`, which would replace
// `@/lib/ppr-token` for the entire test process and break its own test file.
@@ -290,67 +250,18 @@ describe('getPPRStaticSiteScopeContext', () => {
expect(context).toMatchObject({
apiToken,
revisionId: 'ppr-revision-id',
revision: 'ppr-revision-id',
});
});
});
describe('getPPRStaticSiteContext', () => {
/** Run `fn` the way Next runs a request: under a work store of its own. */
function inRequest<T>(fn: () => Promise<T>): Promise<T> {
return workAsyncStorage.run({} as WorkStore, fn);
}
it('uses the supplied API token without resolving published content again', async () => {
const { context } = await getPPRStaticSiteContext(getPPRRouteParams(routeParams), 'body');
type FetchedWith = { pprScope: string; tokenScope: string };
/** Shape produced by the stubs above. */
type ComposedContext = {
apiToken: string;
siteSpace: { id: string };
revisionId: string;
revision: { fetchedWith: FetchedWith };
dataFetcher: { fetchedWith: FetchedWith };
};
it('resolves the header from its own params', async () => {
const { context } = await getPPRStaticSiteContext(
await getPPRHeaderRouteParams(routeParams),
'header'
);
expect(context).toMatchObject({ siteSpace: 'default-site-space-id' });
expect(getTokenScope((context as unknown as { apiToken: string }).apiToken)).toBe('site');
});
it('composes the site scope of the layout, the revision of the TOC and its own fetcher', async () => {
await inRequest(async () => {
const { context } = await getPPRStaticSiteContext(
await getPPRPageRouteParams(routeParams),
'body'
);
const composed = context as unknown as ComposedContext;
// Same call as the layout: site token, visited location rather than the defaults.
expect(getTokenScope(composed.apiToken)).toBe('site');
expect(composed.siteSpace.id).toBe('page-site-space-id');
// Same fetch as the table of contents.
expect(composed.revisionId).toBe('ppr-revision-id');
expect(composed.revision.fetchedWith).toEqual({
pprScope: 'toc',
tokenScope: 'revision',
});
// Anything else the body reads goes through its own token.
expect(composed.dataFetcher.fetchedWith).toEqual({
pprScope: 'body',
tokenScope: 'page',
});
expect(context).toMatchObject({
apiToken,
revision: 'ppr-revision-id',
});
});
it('fails outside a PPR request', async () => {
const pageParams = await inRequest(() => getPPRPageRouteParams(routeParams));
await expect(getPPRStaticSiteContext(pageParams, 'body')).rejects.toThrow(
'outside a PPR request'
);
});
});
packages/gitbook/src/app/utils.ts
+1 -58
View File
@@ -14,13 +14,10 @@ import {
type SiteURLData,
fetchSiteContextByURLLookup,
fetchSiteScopeContextByURLLookup,
fetchSpaceContextByIds,
getBaseContext,
mergeSiteScopeAndSpaceContext,
} from '@/lib/context';
import { getDynamicCustomizationSettings } from '@/lib/customization';
import { PPR_TOKEN_SCOPE, type PPRTokenScope, exchangePPRToken } from '@/lib/ppr-token';
import { createServerContextValue } from '@/lib/server-context';
export type RouteParamMode = 'url-host' | 'url';
@@ -184,32 +181,12 @@ export function getSiteURLDataFromParams(params: RouteLayoutParams): SiteURLData
}
}
/**
* Raw params of the PPR request, shared with the cached components. They read them from here rather
* than from their props, which would put the site and revision tokens in their cache key.
*/
const pprRequestParams = createServerContextValue<PPRRouteLayoutParams>('ppr-request-params');
function getPPRRequestParams(): PPRRouteLayoutParams {
const params = pprRequestParams.read();
if (!params) {
throw new Error(
'PPR component rendered outside a PPR request: the route entry must project its params with getPPRRouteParams first'
);
}
return params;
}
export function getPPRRouteParams(params: PPRRouteParams): RouteParams;
export function getPPRRouteParams(params: PPRRouteLayoutParams): RouteLayoutParams;
/**
* Project PPR route params for the current page, without PPR-only cache inputs.
* Every route entry goes through here before rendering a cached component, so this is also where
* the raw params are shared with them.
*/
export function getPPRRouteParams(params: PPRRouteLayoutParams): RouteLayoutParams {
pprRequestParams.provide(params);
const { revisionId, revalidationId, pprDefaults: _, ...routeParams } = params;
const siteURLData = getSiteURLDataFromParams(params);
@@ -332,43 +309,9 @@ export async function getPPRVisitorAuthClaims(
/**
* Get the static context for a PPR component. The scope partitions the cache entries and scopes
* the tags they emit, so the component and its data are revalidated as one unit.
*
* The context is composed the way the shell is: the site scope of the layout (site token, visited
* location) and the space and revision of the table of contents (revision token). Only the data
* fetcher is the component's own, so what it reads beyond that is narrowed to its scope. The nested
* fetches run inside the cache fill, so the entry is tagged with the data it renders.
*/
export async function getPPRStaticSiteContext(params: RouteLayoutParams, pprScope: PPRCacheScope) {
if (pprScope === 'header') {
// The header renders the default variant, so its site scope is parsed for another location
// than the layout's. Its site fetch is still shared with it: same token, same scope.
return getStaticSiteContext(params, { pprScope });
}
const requestParams = getPPRRequestParams();
const [siteParams, tableOfContentsParams] = await Promise.all([
getPPRSiteRouteParams(requestParams),
getPPRTableOfContentsRouteParams(requestParams),
]);
const { baseContext, decoded } = getStaticBaseContext(params, { pprScope });
const tableOfContents = getStaticBaseContext(tableOfContentsParams, { pprScope: 'toc' });
const [{ context: siteScope }, spaceContext] = await Promise.all([
getStaticSiteScopeContext(siteParams, { pprScope: 'header' }),
fetchSpaceContextByIds(tableOfContents.baseContext, {
space: tableOfContents.siteURLData.space,
shareKey: tableOfContents.siteURLData.shareKey,
changeRequest: tableOfContents.siteURLData.changeRequest,
revision: tableOfContents.siteURLData.revision,
}),
]);
return {
context: mergeSiteScopeAndSpaceContext(siteScope, spaceContext, {
dataFetcher: baseContext.dataFetcher,
}),
visitorAuthClaims: getVisitorAuthClaimsFromToken(decoded),
};
return getStaticSiteContext(params, { pprScope });
}
/**
packages/gitbook/src/components/SitePage/PPRSitePage.tsx
-5
View File
@@ -24,11 +24,6 @@ import {
// cache key of every data fetcher, so the tags they emit are scoped too and propagate up to the
// entry here — making the component and the data it read a single revalidatable unit. No explicit
// `cacheTag` is needed, and adding one would only duplicate a propagated tag.
//
// Only the params of the component are in its cache key. The site scope of the layout and the
// revision of the table of contents reach the fill through the server context, so a component of the
// `toc` or `body` scope renders the same site and revision as the shell. The header is the exception:
// it renders the default variant, so it resolves its own.
/**
* Render the header from cache without carrying a request-scoped data fetcher into the cache key.
packages/gitbook/src/lib/context.test.ts
-29
View File
@@ -11,9 +11,7 @@ import {
type GitBookBaseContext,
fetchSiteContextByIds,
fetchSiteScopeContextByIds,
fetchSpaceContextByIds,
filterSectionsAndGroupsWithHiddenSiteSpaces,
mergeSiteScopeAndSpaceContext,
} from './context';
import { createLinker } from './links';
@@ -152,30 +150,3 @@ describe('fetchSiteContextByIds', () => {
expect(context.revisionId).toBe('space-revision-id');
});
});
describe('mergeSiteScopeAndSpaceContext', () => {
it('keeps the supplied data fetcher over the ones of both contexts', async () => {
const siteBaseContext = getBaseContext();
const spaceBaseContext = getBaseContext();
const dataFetcher = getBaseContext().dataFetcher;
const [siteScope, spaceContext] = await Promise.all([
fetchSiteScopeContextByIds(siteBaseContext, { ...ids, revision: 'revision-id' }),
fetchSpaceContextByIds(spaceBaseContext, {
space: 'space-id',
shareKey: undefined,
changeRequest: undefined,
revision: 'revision-id',
}),
]);
const context = mergeSiteScopeAndSpaceContext(siteScope, spaceContext, { dataFetcher });
expect(context.dataFetcher).toBe(dataFetcher);
expect(context.dataFetcher).not.toBe(siteBaseContext.dataFetcher);
expect(context.dataFetcher).not.toBe(spaceBaseContext.dataFetcher);
expect(context.site.id).toBe('site-id');
expect(context.revision).toBe(revision as unknown as typeof context.revision);
expect(context.revisionId).toBe('revision-id');
expect(context.locale).toBe(TranslationLanguage.Fr);
});
});
packages/gitbook/src/lib/context.ts
-17
View File
@@ -540,26 +540,9 @@ export async function fetchSiteContextByIds(
fetchSpaceContextByIds(baseContext, ids),
]);
return mergeSiteScopeAndSpaceContext(siteScope, spaceContext, {
dataFetcher: baseContext.dataFetcher,
});
}
/**
* Merge a site scope with the space context it is rendered with. The data fetcher is explicit: the
* two may have been resolved with different tokens, and the merged context must fetch through
* neither of them by accident.
*/
export function mergeSiteScopeAndSpaceContext(
siteScope: Omit<GitBookSiteScopeContext, 'revisionId'>,
spaceContext: GitBookSpaceContext,
options: { dataFetcher: GitBookDataFetcher }
): GitBookSiteContext {
return {
...spaceContext,
...siteScope,
dataFetcher: options.dataFetcher,
revisionId: spaceContext.revisionId,
locale: siteScope.siteSpace.space.language ?? spaceContext.locale,
linker: getLinkerForSiteSpace(
siteScope.linker,
packages/gitbook/src/lib/ppr-token.test.ts
+2 -17
View File
@@ -1,8 +1,4 @@
import { afterEach, describe, expect, it, mock } from 'bun:test';
import {
type WorkStore,
workAsyncStorage,
} from 'next/dist/server/app-render/work-async-storage.external';
import { PPR_TOKEN_SCOPE, exchangePPRToken } from './ppr-token';
import { DataFetcherError } from '@/lib/data/errors';
@@ -72,6 +68,8 @@ describe('exchangePPRToken', () => {
expect((error as DataFetcherError).code).toBe(502);
});
// Request-level memoization is `React.cache`, which is inert outside a render scope and so
// cannot be exercised here. What is asserted instead: each scope is a distinct exchange.
it('exchanges each scope separately', async () => {
const calls = mockFetch(() => Response.json({ token: 'exchanged' }));
@@ -85,17 +83,4 @@ describe('exchangePPRToken', () => {
{ token: 'shared-token', scope: 'revision' },
]);
});
// The memo lives on the server context, keyed on the work store of the request, so it is also
// what a cache fill sees.
it('reuses an exchange within a request', async () => {
const calls = mockFetch(() => Response.json({ token: 'exchanged' }));
await workAsyncStorage.run({} as WorkStore, async () => {
await exchangePPRToken('shared-token', 'site');
await exchangePPRToken('shared-token', 'site');
});
expect(calls).toHaveLength(1);
});
});
packages/gitbook/src/lib/ppr-token.ts
+3 -4
View File
@@ -1,8 +1,8 @@
import 'server-only';
import { cache } from '@/lib/cache';
import type { PPRCacheScope } from '@/lib/cache-tags';
import { DataFetcherError } from '@/lib/data/errors';
import { GITBOOK_EXCHANGE_TOKEN_URL } from '@/lib/env';
import { serverCache } from '@/lib/server-context';
import { trace } from '@/lib/tracing';
/**
@@ -23,10 +23,9 @@ export const PPR_TOKEN_SCOPE: Record<PPRCacheScope, PPRTokenScope> = {
* narrowed to `scope`. The API only understands the latter, and narrowing is what lets components
* sharing a scope share a cache entry: the token is part of their cache key.
*
* Memoized on the server context, so the cache fills reuse the exchanges of the route entries, but
* never persisted — an exchanged token is a credential.
* Memoized per request, but never persisted — an exchanged token is a credential.
*/
export const exchangePPRToken = serverCache(
export const exchangePPRToken = cache(
async (token: string, scope: PPRTokenScope): Promise<string> => {
return trace(`exchangePPRToken(${scope})`, async () => {
const response = await fetchExchangedToken(token, scope);
packages/gitbook/src/lib/server-context.test.ts
-83
View File
@@ -1,83 +0,0 @@
import { describe, expect, it, mock } from 'bun:test';
import {
type WorkStore,
workAsyncStorage,
} from 'next/dist/server/app-render/work-async-storage.external';
mock.module('server-only', () => ({}));
const { createServerContextValue, serverCache } = await import('./server-context');
/** Run `fn` the way Next runs a request: under a work store of its own. */
function inRequest<T>(fn: () => T): T {
return workAsyncStorage.run({} as WorkStore, fn);
}
describe('createServerContextValue', () => {
it('provides a value to the rest of the request, first one wins', () => {
const value = createServerContextValue<string>('first-wins');
inRequest(() => {
expect(value.read()).toBeUndefined();
value.provide('first');
value.provide('second');
expect(value.read()).toBe('first');
});
});
it('isolates requests from each other', () => {
const value = createServerContextValue<string>('isolated');
inRequest(() => value.provide('a'));
inRequest(() => expect(value.read()).toBeUndefined());
});
it('is inert outside a request', () => {
const value = createServerContextValue<string>('inert');
value.provide('a');
expect(value.read()).toBeUndefined();
});
});
describe('serverCache', () => {
it('memoizes calls within a request, non-primitive arguments included', () => {
const fn = mock((input: { id: string }) => ({ id: input.id }));
const cached = serverCache(fn);
inRequest(() => {
const first = cached({ id: 'a' });
expect(cached({ id: 'a' })).toBe(first);
expect(cached({ id: 'b' })).not.toBe(first);
});
expect(fn).toHaveBeenCalledTimes(2);
});
it('does not share results across requests', () => {
const fn = mock(() => ({}));
const cached = serverCache(fn);
const first = inRequest(() => cached());
const second = inRequest(() => cached());
expect(second).not.toBe(first);
expect(fn).toHaveBeenCalledTimes(2);
});
it('does not memoize outside a request', () => {
const fn = mock(() => ({}));
const cached = serverCache(fn);
expect(cached()).not.toBe(cached());
expect(fn).toHaveBeenCalledTimes(2);
});
it('keeps functions with the same source apart', () => {
const a = serverCache(() => 'a');
const b = serverCache(() => 'b');
inRequest(() => {
expect(a()).toBe('a');
expect(b()).toBe('b');
});
});
});
packages/gitbook/src/lib/server-context.ts
-79
View File
@@ -1,79 +0,0 @@
import 'server-only';
import { workAsyncStorage } from 'next/dist/server/app-render/work-async-storage.external';
import { identify } from 'object-identity';
/**
* Request-scoped values that survive the `'use cache'` boundary.
*
* Next runs a cached function in a clean `AsyncLocalStorage` snapshot: the request store and React's
* per-request cache are gone inside it, and the arguments are the only thing left. The one object it
* restores is the `WorkStore` of the request, so anything keyed on it is visible both to the route
* entries and to the cache fills they trigger, without being part of a cache key.
*/
const stores = new WeakMap<object, Map<string, unknown>>();
let cachedFunctions = 0;
/**
* Store of the current request, or undefined outside a Next render (tests, scripts).
*/
function getServerContextStore(): Map<string, unknown> | undefined {
const workStore = workAsyncStorage.getStore();
if (!workStore) {
return undefined;
}
let store = stores.get(workStore);
if (!store) {
store = new Map();
stores.set(workStore, store);
}
return store;
}
/**
* Declare a value provided once per request. `provide` keeps the first value, so callers that may run
* in any order (a layout, a page, their metadata) can all provide it.
*/
export function createServerContextValue<T>(name: string) {
const key = `value:${name}`;
return {
provide(value: T): void {
const store = getServerContextStore();
if (store && !store.has(key)) {
store.set(key, value);
}
},
read(): T | undefined {
return getServerContextStore()?.get(key) as T | undefined;
},
};
}
/**
* Equivalent to `cache` from `@/lib/cache`, but memoized on the server context: a call made inside a
* cache fill reuses the result of the same call made by the layout. Outside a request it is inert.
*/
export function serverCache<Args extends any[], Return>(
fn: (...args: Args) => Return
): (...args: Args) => Return {
// `identify` hashes a function by its source, which two distinct functions can share.
const prefix = `cache:${cachedFunctions++}:`;
return (...args: Args) => {
const store = getServerContextStore();
if (!store) {
return fn(...args);
}
const key = prefix + identify(args);
if (store.has(key)) {
return store.get(key) as Return;
}
const result = fn(...args);
store.set(key, result);
return result;
};
}
packages/gitbook/tests/preload-bun.ts
-7
View File
@@ -1,5 +1,4 @@
import { mock } from 'bun:test';
import { AsyncLocalStorage } from 'node:async_hooks';
/**
* Mock the `server-only` module to avoid errors when running tests as it doesn't work well in Bun
@@ -7,9 +6,3 @@ import { AsyncLocalStorage } from 'node:async_hooks';
mock.module('server-only', () => {
return {};
});
/**
* Next reads `AsyncLocalStorage` from the global it sets up in its node environment, and falls back
* to a fake that throws on `run`. Tests running code under a work store need the real one.
*/
globalThis.AsyncLocalStorage = AsyncLocalStorage;
patches/next@16.3.3.patch
-54
View File
@@ -29,57 +29,3 @@ index d9c2e5e801c91b2edbc0101c56af14c3dda6c42e..0b65253e39cecc67c76e5d69105c56c0
}
function getOutlinedModel(response, reference, parentObject, key, map) {
var path = reference.split(":");
diff --git a/dist/esm/server/app-render/work-async-storage-instance.js b/dist/esm/server/app-render/work-async-storage-instance.js
index ff0edc4aa1a2674eb995361a6c3497d9dbe56ee4..b678b282dddfe00f2a6e9c4b0f3b7c92f80a4e25 100644
--- a/dist/esm/server/app-render/work-async-storage-instance.js
+++ b/dist/esm/server/app-render/work-async-storage-instance.js
@@ -1,4 +1,20 @@
import { createAsyncLocalStorage } from './async-local-storage';
-export const workAsyncStorageInstance = createAsyncLocalStorage();
+// GitBook patch: share one instance per process. OpenNext inlines a copy of this module into its
+// handler while route chunks require another from node_modules. `'use cache'` fills restore the
+// work store through this storage, so app code reading it inside a fill needs every copy to agree.
+// Only a real storage is shared: a fake created before the global is set must not be pinned.
+const sharedInstanceKey = Symbol.for('next.workAsyncStorageInstance');
+function getWorkAsyncStorageInstance() {
+ const existing = globalThis[sharedInstanceKey];
+ if (existing) {
+ return existing;
+ }
+ const instance = createAsyncLocalStorage();
+ if (typeof globalThis.AsyncLocalStorage !== 'undefined') {
+ globalThis[sharedInstanceKey] = instance;
+ }
+ return instance;
+}
+export const workAsyncStorageInstance = getWorkAsyncStorageInstance();
//# sourceMappingURL=work-async-storage-instance.js.map
diff --git a/dist/server/app-render/work-async-storage-instance.js b/dist/server/app-render/work-async-storage-instance.js
index 1ea9c61d574f2ed2753b0ed94887adaa6c1ecb92..1edb9c339ea55b930a10c5a446f33ce8e541eb7b 100644
--- a/dist/server/app-render/work-async-storage-instance.js
+++ b/dist/server/app-render/work-async-storage-instance.js
@@ -9,6 +9,22 @@ Object.defineProperty(exports, "workAsyncStorageInstance", {
}
});
const _asynclocalstorage = require("./async-local-storage");
-const workAsyncStorageInstance = (0, _asynclocalstorage.createAsyncLocalStorage)();
+// GitBook patch: share one instance per process. OpenNext inlines a copy of this module into its
+// handler while route chunks require another from node_modules. `'use cache'` fills restore the
+// work store through this storage, so app code reading it inside a fill needs every copy to agree.
+// Only a real storage is shared: a fake created before the global is set must not be pinned.
+const sharedInstanceKey = Symbol.for("next.workAsyncStorageInstance");
+function getWorkAsyncStorageInstance() {
+ const existing = globalThis[sharedInstanceKey];
+ if (existing) {
+ return existing;
+ }
+ const instance = (0, _asynclocalstorage.createAsyncLocalStorage)();
+ if (typeof globalThis.AsyncLocalStorage !== "undefined") {
+ globalThis[sharedInstanceKey] = instance;
+ }
+ return instance;
+}
+const workAsyncStorageInstance = getWorkAsyncStorageInstance();
//# sourceMappingURL=work-async-storage-instance.js.map