Skip to content
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

fix(mdx-loader): fix cross-compiler cache randomly loading mdx with client/server envs #10553

Merged
merged 5 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions packages/docusaurus-mdx-loader/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
compileToJSX,
createAssetsExportCode,
extractContentTitleData,
promiseWithResolvers,
} from './utils';
import type {WebpackCompilerName} from '@docusaurus/utils';
import type {Options} from './options';
Expand Down Expand Up @@ -138,30 +139,76 @@ async function loadMDXWithCaching({
options: Options;
compilerName: WebpackCompilerName;
}): Promise<string> {
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<string>();
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(
Expand Down
5 changes: 4 additions & 1 deletion packages/docusaurus-mdx-loader/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<MDXOptions> & {
markdownConfig: MarkdownConfig;
Expand All @@ -25,5 +26,7 @@ export type Options = Partial<MDXOptions> & {

// Will usually be created by "createMDXLoaderItem"
processors?: SimpleProcessors;
crossCompilerCache?: Map<string, Promise<string>>; // MDX => Promise<JSX> cache
crossCompilerCache?: Map<string, CrossCompilerCacheEntry>; // MDX => Promise<JSX> cache
};

type CrossCompilerCacheEntry = PromiseWithResolvers<string>;
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions packages/docusaurus-mdx-loader/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,20 @@ export async function compileToJSX({
);
}
}

// TODO Docusaurus v4, remove temporary polyfill when upgrading to Node 22+
export interface PromiseWithResolvers<T> {
promise: Promise<T>;
resolve: (value: T | PromiseLike<T>) => void;
reject: (reason?: any) => void;
}
// TODO Docusaurus v4, remove temporary polyfill when upgrading to Node 22+
export function promiseWithResolvers<T>(): PromiseWithResolvers<T> {
// @ts-expect-error: it's fine
const out: PromiseWithResolvers<T> = {};
out.promise = new Promise((resolve, reject) => {
out.resolve = resolve;
out.reject = reject;
});
return out;
}