Skip to content
Draft
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
70 changes: 70 additions & 0 deletions src/dev-generation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type CreateReactRouterDevRuntimeOptions = {
buildPlan: ReactRouterDevBuildPlan;
onEvaluationError: (error: Error) => void;
onCssAssetOwnershipChanged?: () => void;
onRouteManifestChanged?: () => void;
onWarning?: (message: string) => void;
};

Expand Down Expand Up @@ -194,6 +195,53 @@ const hasOnlyCssAssetOwnershipChanges = (
});
};

const normalizeManifestForRouteMetadataCheck = (
manifest: ReactRouterDevManifestSet[string]
) =>
Object.fromEntries(
Object.entries(manifest.routes ?? {})
.sort(([left], [right]) => left.localeCompare(right))
.map(([routeId, route]) => [
routeId,
{
caseSensitive: route.caseSensitive,
clientActionModule: route.clientActionModule,
clientLoaderModule: route.clientLoaderModule,
clientMiddlewareModule: route.clientMiddlewareModule,
errorBoundary: route.hasErrorBoundary,
hasAction: route.hasAction,
hasClientAction: route.hasClientAction,
hasClientLoader: route.hasClientLoader,
hasClientMiddleware: route.hasClientMiddleware,
hasDefaultExport: route.hasDefaultExport,
hasLoader: route.hasLoader,
hydrateFallbackModule: route.hydrateFallbackModule,
id: route.id,
index: route.index,
parentId: route.parentId,
path: route.path,
},
])
);

const hasRouteManifestMetadataChanges = (
previous: ReactRouterDevManifestSet,
next: ReactRouterDevManifestSet
): boolean => {
const previousEntryNames = Object.keys(previous).sort();
const nextEntryNames = Object.keys(next).sort();
if (previousEntryNames.join('\0') !== nextEntryNames.join('\0')) {
return true;
}
return previousEntryNames.some(entryName => {
const previousRoutes = normalizeManifestForRouteMetadataCheck(
previous[entryName]
);
const nextRoutes = normalizeManifestForRouteMetadataCheck(next[entryName]);
return JSON.stringify(previousRoutes) !== JSON.stringify(nextRoutes);
});
};

const createDeferred = <T>(): Deferred<T> => {
let resolve!: (value: T) => void;
let reject!: (error: Error) => void;
Expand All @@ -212,6 +260,7 @@ export const createReactRouterDevRuntime = ({
buildPlan,
onEvaluationError,
onCssAssetOwnershipChanged = () => undefined,
onRouteManifestChanged = () => undefined,
onWarning = () => undefined,
}: CreateReactRouterDevRuntimeOptions): ReactRouterDevRuntime => {
let nextAttemptId = 1;
Expand All @@ -237,6 +286,17 @@ export const createReactRouterDevRuntime = ({
}
};

const notifyRouteManifestChanged = (): void => {
try {
onRouteManifestChanged();
} catch (cause) {
const reason = cause instanceof Error ? cause.message : String(cause);
onWarning(
`[rsbuild-plugin-react-router] Failed to notify the browser after route manifest metadata changed: ${reason}`
);
}
};

const uniqueEntryNames = new Set(buildPlan.entryNames);
if (
uniqueEntryNames.size !== buildPlan.entryNames.length ||
Expand Down Expand Up @@ -450,6 +510,13 @@ export const createReactRouterDevRuntime = ({
previous.web.manifestsByEntryName,
manifestsByEntryName
);
const routeManifestMetadataChanged =
!!previous &&
webChanged &&
hasRouteManifestMetadataChanges(
previous.web.manifestsByEntryName,
manifestsByEntryName
);
const reusePreviousNodeBuild = !!previous && cssOnlyWebManifestChange;

if (
Expand Down Expand Up @@ -517,6 +584,9 @@ export const createReactRouterDevRuntime = ({
}
reloadAfterCssRemoval = false;
}
if (routeManifestMetadataChanged) {
notifyRouteManifestChanged();
}
} catch (cause) {
rejectAttempt(
attemptId,
Expand Down
6 changes: 6 additions & 0 deletions src/dev-runtime-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ export const createReactRouterDevRuntimeController = ({
}
server.sockWrite('full-reload', { path: '*' });
},
onRouteManifestChanged() {
if (sessions.getActiveBinding()?.runtime !== runtime) {
return;
}
server.sockWrite('full-reload', { path: '*' });
},
onWarning: message => api.logger.warn(message),
});
const binding = sessions.createBinding(server, runtime);
Expand Down
58 changes: 56 additions & 2 deletions tests/dev-generation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ const createBuild = (
const createRouteManifest = (
id: string,
css: string[],
imports: string[] = []
imports: string[] = [],
overrides: Partial<ReactRouterDevManifest['routes'][string]> = {}
): ReactRouterDevManifest['routes'][string] => ({
id,
module: `/${id}.js`,
Expand All @@ -81,6 +82,7 @@ const createRouteManifest = (
hasErrorBoundary: false,
imports,
css,
...overrides,
});

const createDevManifest = (
Expand Down Expand Up @@ -150,7 +152,10 @@ const captureWeb = (

const createHarness = (
loadBundle: (entryName: string) => Promise<unknown> | unknown,
options: { onCssAssetOwnershipChanged?: () => void } = {}
options: {
onCssAssetOwnershipChanged?: () => void;
onRouteManifestChanged?: () => void;
} = {}
) => {
const errors: Error[] = [];
const warnings: string[] = [];
Expand All @@ -168,6 +173,7 @@ const createHarness = (
},
onEvaluationError: error => errors.push(error),
onCssAssetOwnershipChanged: options.onCssAssetOwnershipChanged,
onRouteManifestChanged: options.onRouteManifestChanged,
onWarning: warning => warnings.push(warning),
});
return { errors, loadBundle: loadBundleMock, runtime, server, warnings };
Expand Down Expand Up @@ -376,6 +382,54 @@ describe('React Router development runtime', () => {
});
});

it('notifies when route export metadata changes', async () => {
const onRouteManifestChanged = rstest.fn();
const { runtime } = createHarness(() => createBuild('build'), {
onRouteManifestChanged,
});
const firstWeb = createCompilation('web');
const firstNode = createCompilation('node');

runtime.beginAttempt();
runtime.captureWeb(firstWeb, {
'static/js/app': {
...createDevManifest('base'),
routes: {
'routes/about': createRouteManifest('routes/about', [], [], {
hasClientLoader: false,
}),
},
},
});
await runtime.finishAttempt(
createGraphStats(firstWeb, firstNode),
noKnownChanges,
graphIdentity(firstWeb, firstNode)
);

const nextWeb = createCompilation('web');
const nextNode = createCompilation('node');
runtime.beginAttempt();
runtime.captureWeb(nextWeb, {
'static/js/app': {
...createDevManifest('next'),
routes: {
'routes/about': createRouteManifest('routes/about', [], [], {
hasClientLoader: true,
clientLoaderModule: '/routes/about.clientLoader.js',
}),
},
},
});
await runtime.finishAttempt(
createGraphStats(nextWeb, nextNode),
noKnownChanges,
graphIdentity(nextWeb, nextNode)
);

expect(onRouteManifestChanged).toHaveBeenCalledOnce();
});

it('notifies when css ownership is re-added after a removal', async () => {
const onCssAssetOwnershipChanged = rstest.fn();
const { runtime } = createHarness(() => createBuild('build'), {
Expand Down
51 changes: 50 additions & 1 deletion tests/dev-runtime-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ const createBuild = (

const createRouteManifest = (
id: string,
css: string[]
css: string[],
overrides: Partial<ReactRouterDevManifest['routes'][string]> = {}
): ReactRouterDevManifest['routes'][string] => ({
id,
module: `/${id}.js`,
Expand All @@ -60,6 +61,7 @@ const createRouteManifest = (
hasErrorBoundary: false,
imports: [],
css,
...overrides,
});

const createManifest = (
Expand Down Expand Up @@ -565,6 +567,53 @@ describe('React Router development runtime controller', () => {
});
});

it('hard reloads when route export metadata changes', async () => {
const { callbacks, controller, loadBundle, server } = createHarness();
loadBundle.mockImplementation(() => createBuild('base'));
const web = createCompiler('web');
const node = createCompiler('node');
await callbacks.start({ server });
callbacks.created({
compiler: { compilers: [web.compiler, node.compiler] },
});
callbacks.before();
const baseWeb = web.compile();
controller.captureWeb(baseWeb, {
'static/js/app': {
...createManifest('web-base')['static/js/app'],
routes: {
'routes/about': createRouteManifest('routes/about', [], {
hasClientLoader: false,
}),
},
},
});
web.complete(baseWeb);
const baseNode = node.compile();
await callbacks.after({ stats: createGraphStats(baseWeb, baseNode) });

callbacks.before();
const nextWeb = web.compile();
controller.captureWeb(nextWeb, {
'static/js/app': {
...createManifest('web-next')['static/js/app'],
routes: {
'routes/about': createRouteManifest('routes/about', [], {
hasClientLoader: true,
clientLoaderModule: '/routes/about.clientLoader.js',
}),
},
},
});
web.complete(nextWeb);
const nextNode = node.compile();
await callbacks.after({ stats: createGraphStats(nextWeb, nextNode) });

expect(server.sockWrite).toHaveBeenCalledWith('full-reload', {
path: '*',
});
});

it('publishes a safe node-only compile after the aggregate pre-hook', async () => {
const { callbacks, controller, loadBundle, server } = createHarness();
let build = createBuild('base');
Expand Down
Loading