From bb57d58b4083ed54f2ed8c1f4679458202960921 Mon Sep 17 00:00:00 2001 From: sebastien Date: Thu, 3 Oct 2024 22:20:25 +0200 Subject: [PATCH 1/5] fix cross-compiler cache --- packages/docusaurus-mdx-loader/src/loader.ts | 65 ++++++++++++++----- packages/docusaurus-mdx-loader/src/options.ts | 5 +- .../src/remark/unusedDirectives/index.ts | 1 + packages/docusaurus-mdx-loader/src/utils.ts | 17 +++++ 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/packages/docusaurus-mdx-loader/src/loader.ts b/packages/docusaurus-mdx-loader/src/loader.ts index 546d4390dccf..837b6f7803d1 100644 --- a/packages/docusaurus-mdx-loader/src/loader.ts +++ b/packages/docusaurus-mdx-loader/src/loader.ts @@ -17,6 +17,7 @@ import { compileToJSX, createAssetsExportCode, extractContentTitleData, + promiseWithResolvers, } from './utils'; import type {WebpackCompilerName} from '@docusaurus/utils'; import type {Options} from './options'; @@ -144,24 +145,56 @@ async function loadMDXWithCaching({ // - the same content can be processed differently (versioned docs links) const cacheKey = resource; - const cachedPromise = options.crossCompilerCache?.get(cacheKey); - if (cachedPromise) { - // We can clean up the cache and free memory here - // We know there are only 2 compilations for the same file - // Note: once we introduce RSCs we'll probably have 3 compilations - // Note: we can't use string keys in WeakMap - // But we could eventually use WeakRef for the values + // We can clean up the cache and free memory after cache entry consumption + // We know there are only 2 compilations for the same file + // Note: once we introduce RSCs we'll probably have 3 compilations + // Note: we can't use string keys in WeakMap + // But we could eventually use WeakRef for the values + function deleteCacheEntry() { options.crossCompilerCache?.delete(cacheKey); - return cachedPromise; } - const promise = loadMDX({ - fileContent, - filePath, - options, - compilerName, - }); - options.crossCompilerCache?.set(cacheKey, promise); - return promise; + + const cacheEntry = options.crossCompilerCache?.get(cacheKey); + + // When deduplicating client/server compilations, we always use the client + // compilation and not the server compilation + // This is important because the server compilation usually skips some steps + // Notably: the server compilation does not emit file-loader assets + // Using the server compilation otherwise leads to broken images + // See https://github.com/facebook/docusaurus/issues/10544#issuecomment-2390943794 + if (compilerName === 'client') { + const promise = loadMDX({ + fileContent, + filePath, + options, + compilerName, + }); + if (cacheEntry) { + promise.then(cacheEntry.resolve, cacheEntry.reject); + deleteCacheEntry(); + } else { + const noop = () => { + throw new Error('this should never be called'); + }; + options.crossCompilerCache?.set(cacheKey, { + promise, + resolve: noop, + reject: noop, + }); + } + return promise; + } else if (compilerName === 'server') { + if (cacheEntry) { + deleteCacheEntry(); + return cacheEntry.promise; + } else { + const {promise, resolve, reject} = promiseWithResolvers(); + options.crossCompilerCache?.set(cacheKey, {promise, resolve, reject}); + return promise; + } + } else { + throw new Error(`Unexpected compilerName=${compilerName}`); + } } export async function mdxLoader( diff --git a/packages/docusaurus-mdx-loader/src/options.ts b/packages/docusaurus-mdx-loader/src/options.ts index 0c8c7fe4ceeb..ed199dc4afab 100644 --- a/packages/docusaurus-mdx-loader/src/options.ts +++ b/packages/docusaurus-mdx-loader/src/options.ts @@ -8,6 +8,7 @@ import type {MDXOptions, SimpleProcessors} from './processor'; import type {MarkdownConfig} from '@docusaurus/types'; import type {ResolveMarkdownLink} from './remark/resolveMarkdownLinks'; +import type {PromiseWithResolvers} from './utils'; export type Options = Partial & { markdownConfig: MarkdownConfig; @@ -25,5 +26,7 @@ export type Options = Partial & { // Will usually be created by "createMDXLoaderItem" processors?: SimpleProcessors; - crossCompilerCache?: Map>; // MDX => Promise cache + crossCompilerCache?: Map; // MDX => Promise cache }; + +type CrossCompilerCacheEntry = PromiseWithResolvers; diff --git a/packages/docusaurus-mdx-loader/src/remark/unusedDirectives/index.ts b/packages/docusaurus-mdx-loader/src/remark/unusedDirectives/index.ts index d2cb5928d63f..b4641e522b71 100644 --- a/packages/docusaurus-mdx-loader/src/remark/unusedDirectives/index.ts +++ b/packages/docusaurus-mdx-loader/src/remark/unusedDirectives/index.ts @@ -155,6 +155,7 @@ const plugin: Plugin = function plugin(this: Processor): Transformer { // We only enable these warnings for the client compiler // This avoids emitting duplicate warnings in prod mode // Note: the client compiler is used in both dev/prod modes + // Also: the client compiler is what gets used when using crossCompilerCache if (file.data.compilerName === 'client') { logUnusedDirectivesWarning({ directives: unusedDirectives, diff --git a/packages/docusaurus-mdx-loader/src/utils.ts b/packages/docusaurus-mdx-loader/src/utils.ts index 8941e2e90846..eddca6ac56fc 100644 --- a/packages/docusaurus-mdx-loader/src/utils.ts +++ b/packages/docusaurus-mdx-loader/src/utils.ts @@ -132,3 +132,20 @@ export async function compileToJSX({ ); } } + +// TODO Docusaurus v4, remove temporary polyfill when upgrading to Node 22+ +export interface PromiseWithResolvers { + promise: Promise; + resolve: (value: T | PromiseLike) => void; + reject: (reason?: any) => void; +} +// TODO Docusaurus v4, remove temporary polyfill when upgrading to Node 22+ +export function promiseWithResolvers(): PromiseWithResolvers { + // @ts-expect-error: it's fine + const out: PromiseWithResolvers = {}; + out.promise = new Promise((resolve, reject) => { + out.resolve = resolve; + out.reject = reject; + }); + return out; +} From edc71b728e2f483bc6a55a977b33a06703c2596e Mon Sep 17 00:00:00 2001 From: sebastien Date: Thu, 3 Oct 2024 22:36:59 +0200 Subject: [PATCH 2/5] add loadMDXWithCaching comment --- packages/docusaurus-mdx-loader/src/loader.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/docusaurus-mdx-loader/src/loader.ts b/packages/docusaurus-mdx-loader/src/loader.ts index 837b6f7803d1..78120ac9f499 100644 --- a/packages/docusaurus-mdx-loader/src/loader.ts +++ b/packages/docusaurus-mdx-loader/src/loader.ts @@ -162,6 +162,10 @@ async function loadMDXWithCaching({ // Notably: the server compilation does not emit file-loader assets // Using the server compilation otherwise leads to broken images // See https://github.com/facebook/docusaurus/issues/10544#issuecomment-2390943794 + // See https://github.com/facebook/docusaurus/pull/10553 + // TODO a problem with this: server bundle will use client inline loaders + // This means server bundle will use ?emit=true for assets + // We should try to get rid of inline loaders to cleanup this caching logic if (compilerName === 'client') { const promise = loadMDX({ fileContent, From 2694260e8dbf822b4d86ac757a23faa8001dd1a5 Mon Sep 17 00:00:00 2001 From: sebastien Date: Thu, 3 Oct 2024 22:40:29 +0200 Subject: [PATCH 3/5] add loadMDXWithCaching comment --- packages/docusaurus-mdx-loader/src/loader.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/docusaurus-mdx-loader/src/loader.ts b/packages/docusaurus-mdx-loader/src/loader.ts index 78120ac9f499..9bd9880ecda8 100644 --- a/packages/docusaurus-mdx-loader/src/loader.ts +++ b/packages/docusaurus-mdx-loader/src/loader.ts @@ -150,9 +150,7 @@ async function loadMDXWithCaching({ // Note: once we introduce RSCs we'll probably have 3 compilations // Note: we can't use string keys in WeakMap // But we could eventually use WeakRef for the values - function deleteCacheEntry() { - options.crossCompilerCache?.delete(cacheKey); - } + const deleteCacheEntry = () => options.crossCompilerCache?.delete(cacheKey); const cacheEntry = options.crossCompilerCache?.get(cacheKey); @@ -187,7 +185,9 @@ async function loadMDXWithCaching({ }); } return promise; - } else if (compilerName === 'server') { + } + // Server compilation always uses the result of the client compilation above + else if (compilerName === 'server') { if (cacheEntry) { deleteCacheEntry(); return cacheEntry.promise; From 8a8f94d6404020f9e68b84960e6678f35e8748fe Mon Sep 17 00:00:00 2001 From: sebastien Date: Thu, 3 Oct 2024 22:57:46 +0200 Subject: [PATCH 4/5] fix bug when crossCompilerCache is disabled --- .../src/createMDXLoader.ts | 6 +++++- packages/docusaurus-mdx-loader/src/loader.ts | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/docusaurus-mdx-loader/src/createMDXLoader.ts b/packages/docusaurus-mdx-loader/src/createMDXLoader.ts index 9bbafad5f7da..9f23f9d9a0dd 100644 --- a/packages/docusaurus-mdx-loader/src/createMDXLoader.ts +++ b/packages/docusaurus-mdx-loader/src/createMDXLoader.ts @@ -33,7 +33,11 @@ async function normalizeOptions( // We don't want to cache in dev mode (docusaurus start) // We only have multiple compilers in production mode (docusaurus build) // TODO wrong but good enough for now (example: "docusaurus build --dev") - if (options.useCrossCompilerCache && process.env.NODE_ENV === 'production') { + if ( + process.env.DOCUSAURUS_AB_BENCHMARK === 'true' && + options.useCrossCompilerCache && + process.env.NODE_ENV === 'production' + ) { options = { ...options, crossCompilerCache: new Map(), diff --git a/packages/docusaurus-mdx-loader/src/loader.ts b/packages/docusaurus-mdx-loader/src/loader.ts index 9bd9880ecda8..f87a19e4872c 100644 --- a/packages/docusaurus-mdx-loader/src/loader.ts +++ b/packages/docusaurus-mdx-loader/src/loader.ts @@ -139,6 +139,16 @@ async function loadMDXWithCaching({ options: Options; compilerName: WebpackCompilerName; }): Promise { + const {crossCompilerCache} = options; + if (!crossCompilerCache) { + return loadMDX({ + fileContent, + filePath, + options, + compilerName, + }); + } + // Note we "resource" as cache key, not "filePath" nor "fileContent" // This is because: // - the same file can be compiled in different variants (blog.mdx?truncated) @@ -150,9 +160,9 @@ async function loadMDXWithCaching({ // Note: once we introduce RSCs we'll probably have 3 compilations // Note: we can't use string keys in WeakMap // But we could eventually use WeakRef for the values - const deleteCacheEntry = () => options.crossCompilerCache?.delete(cacheKey); + const deleteCacheEntry = () => crossCompilerCache.delete(cacheKey); - const cacheEntry = options.crossCompilerCache?.get(cacheKey); + const cacheEntry = crossCompilerCache?.get(cacheKey); // When deduplicating client/server compilations, we always use the client // compilation and not the server compilation @@ -178,7 +188,7 @@ async function loadMDXWithCaching({ const noop = () => { throw new Error('this should never be called'); }; - options.crossCompilerCache?.set(cacheKey, { + crossCompilerCache.set(cacheKey, { promise, resolve: noop, reject: noop, @@ -193,7 +203,7 @@ async function loadMDXWithCaching({ return cacheEntry.promise; } else { const {promise, resolve, reject} = promiseWithResolvers(); - options.crossCompilerCache?.set(cacheKey, {promise, resolve, reject}); + crossCompilerCache.set(cacheKey, {promise, resolve, reject}); return promise; } } else { From d815444c28cbd0c7b214c2b31fb4d714505c1507 Mon Sep 17 00:00:00 2001 From: sebastien Date: Thu, 3 Oct 2024 22:58:13 +0200 Subject: [PATCH 5/5] revert ab test --- packages/docusaurus-mdx-loader/src/createMDXLoader.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/docusaurus-mdx-loader/src/createMDXLoader.ts b/packages/docusaurus-mdx-loader/src/createMDXLoader.ts index 9f23f9d9a0dd..9bbafad5f7da 100644 --- a/packages/docusaurus-mdx-loader/src/createMDXLoader.ts +++ b/packages/docusaurus-mdx-loader/src/createMDXLoader.ts @@ -33,11 +33,7 @@ async function normalizeOptions( // We don't want to cache in dev mode (docusaurus start) // We only have multiple compilers in production mode (docusaurus build) // TODO wrong but good enough for now (example: "docusaurus build --dev") - if ( - process.env.DOCUSAURUS_AB_BENCHMARK === 'true' && - options.useCrossCompilerCache && - process.env.NODE_ENV === 'production' - ) { + if (options.useCrossCompilerCache && process.env.NODE_ENV === 'production') { options = { ...options, crossCompilerCache: new Map(),