From 9e473bd0805d2e33c17fe9ca08c25852f91ea2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lorber?= Date: Thu, 3 Oct 2024 23:20:57 +0200 Subject: [PATCH] fix(mdx-loader): fix cross-compiler cache randomly loading mdx with client/server envs (#10553) --- packages/docusaurus-mdx-loader/src/loader.ts | 81 +++++++++++++++---- 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, 86 insertions(+), 18 deletions(-) diff --git a/packages/docusaurus-mdx-loader/src/loader.ts b/packages/docusaurus-mdx-loader/src/loader.ts index 546d4390dccf..f87a19e4872c 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'; @@ -138,30 +139,76 @@ 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) // - 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 - options.crossCompilerCache?.delete(cacheKey); - return cachedPromise; + // 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 + const deleteCacheEntry = () => crossCompilerCache.delete(cacheKey); + + const cacheEntry = 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 + // 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, + filePath, + options, + compilerName, + }); + if (cacheEntry) { + promise.then(cacheEntry.resolve, cacheEntry.reject); + deleteCacheEntry(); + } else { + const noop = () => { + throw new Error('this should never be called'); + }; + crossCompilerCache.set(cacheKey, { + promise, + resolve: noop, + reject: noop, + }); + } + return promise; + } + // Server compilation always uses the result of the client compilation above + else if (compilerName === 'server') { + if (cacheEntry) { + deleteCacheEntry(); + return cacheEntry.promise; + } else { + const {promise, resolve, reject} = promiseWithResolvers(); + crossCompilerCache.set(cacheKey, {promise, resolve, reject}); + return promise; + } + } else { + throw new Error(`Unexpected compilerName=${compilerName}`); } - const promise = loadMDX({ - fileContent, - filePath, - options, - compilerName, - }); - options.crossCompilerCache?.set(cacheKey, promise); - return promise; } 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; +}