diff --git a/.changeset/sharp-routes-heal.md b/.changeset/sharp-routes-heal.md new file mode 100644 index 0000000..1b44496 --- /dev/null +++ b/.changeset/sharp-routes-heal.md @@ -0,0 +1,6 @@ +--- +"rsbuild-plugin-react-router": patch +--- + +Harden route module transforms and development route watching so source maps, +server/client-only modules, and route topology restarts behave consistently. diff --git a/README.md b/README.md index 33b4587..482b790 100644 --- a/README.md +++ b/README.md @@ -104,10 +104,17 @@ pluginReactRouter({ /** * Run route transforms in a worker-thread pool. * Pass `false` to disable or `{ maxWorkers }` to override the default worker count. - * @default true, using `available CPUs - 2` workers. + * @default Automatically enabled for 256+ resolved routes. The automatic + * pool is capped at 8 workers. */ parallelTransforms?: boolean | { maxWorkers?: number }, + /** + * Route-topology notification for programmatic/custom dev servers. + * Recreate the Rsbuild server when this fires. + */ + onRouteTopologyChange?: () => void | Promise, + /** * Enable experimental support for module federation * @default false @@ -277,8 +284,8 @@ export default { } satisfies Config; ``` -For large sites, prerendering defaults to `availableParallelism - 2` concurrent -paths. You can tune prerender concurrency: +Prerendering defaults to one path at a time, matching React Router. You can opt +into concurrent prerendering for large sites: ```ts export default { @@ -302,7 +309,7 @@ If no configuration is provided, the following defaults will be used: federation: false, lazyCompilation: false, logPerformance: false, - parallelTransforms: true // adaptive worker pool + parallelTransforms: undefined // adaptive: workers for 256+ resolved routes } // Router defaults (react-router.config.ts) @@ -314,9 +321,10 @@ If no configuration is provided, the following defaults will be used: } ``` -`parallelTransforms: true` uses worker threads for route builds. The default -worker count is `availableParallelism - 2`. Pass `{ maxWorkers }` to override -that count, or `false` to run route transforms inline. +Route transforms run inline for fewer than 256 resolved routes and use worker +threads for larger route graphs. The automatic worker count is capped at 8. +Pass `true` to force workers, `{ maxWorkers }` (up to 32) to override that +count, or `false` to force inline transforms. For builds with 256+ routes, detailed file-size reporting is compacted to totals by default to avoid gzipping and printing thousands of assets. Set @@ -437,6 +445,12 @@ export default defineConfig(() => { }); ``` +If the server is created programmatically with `createDevServer()`, pass +`onRouteTopologyChange` and use it to recreate that server. Rsbuild's +`reload-server` watcher is owned by the CLI and is not installed by the +programmatic API. The callback is a notification and is not awaited, so it can +safely close the current server as part of the replacement. + When using a custom server, you'll need to: 1. Create a server handler (`server/index.ts`): diff --git a/benchmarks/manifest-performance-methodology.md b/benchmarks/manifest-performance-methodology.md index 6535646..765eb5a 100644 --- a/benchmarks/manifest-performance-methodology.md +++ b/benchmarks/manifest-performance-methodology.md @@ -50,15 +50,17 @@ The harness: 1. builds the plugin package (`pnpm build`) unless `--skip-root-build` is passed; 2. generates deterministic fixtures under `.benchmark/fixtures/`; 3. runs `node node_modules/@rsbuild/core/bin/rsbuild.js build --config rsbuild.config.mjs`; -4. sets `REACT_ROUTER_BENCHMARK_LOG_PERFORMANCE=1`, enabling structured - `[react-router:performance]` plugin logs; +4. keeps plugin instrumentation disabled for canonical end-to-end A/B runs; + pass `--log-performance` for a separate diagnostic run that emits structured + `[react-router:performance]` logs; 5. wraps builds in `/usr/bin/time -v` when available and records user/sys/RSS; 6. writes `.benchmark/results//baseline.json` and `baseline.md`. `rsbuild build --help` in this repo exposes `--log-level`, `--environment`, `--mode`, and `--config`, but no dedicated benchmark/stats/profiling CLI flag. -Use the plugin `logPerformance` reports as the primary plugin-level source of -truth. If low-level Rspack stats are needed later, add them through fixture +Use end-to-end wall time, process CPU, and RSS as the primary comparison +signals. Plugin `logPerformance` reports are diagnostic because their timers +include queueing and add observer overhead. If low-level Rspack stats are needed later, add them through fixture `rsbuild.config.mjs`; do not depend on a non-existent CLI flag. ## Pre-flight commands @@ -163,13 +165,22 @@ because it controls warmup, cleaning, aggregation, and output format. ## Metric checklist -### Already observable from `baseline.json` +### Canonical metrics in `baseline.json` | Metric | Source | Why it matters | | --------------------------- | ------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | | Build wall time | `benchmarks[].summary.wallMs` | End-to-end user-visible build time. | | CPU time | `summary.userMs` + `summary.sysMs` | Less noisy than wall time when the machine has minor scheduling variance. | | Peak RSS | `summary.maxRssKb` | Ensures cache dedup does not regress memory. | + +### Diagnostic metrics with `--log-performance` + +These fields are empty in canonical A/B runs because plugin instrumentation is +disabled by default. Use a separate diagnostic run when operation-level +attribution is needed. + +| Metric | Source | Why it matters | +| --------------------------- | ------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | | Compiler lifecycle | each plugin report's `compilerLifecycleMs` | Plugin setup/build lifecycle timing per compiler environment. | | Transform invocation counts | `pluginOperations[].count` | Counts route/manifest hook invocations. Counts should usually stay stable after dedup; timings should drop. | | Transform cumulative time | `pluginOperations[].totalMs` | Primary signal for expensive plugin work moving out of duplicate paths. | diff --git a/examples/default-template/app/dev-routes.ts b/examples/default-template/app/dev-routes.ts new file mode 100644 index 0000000..42c5f72 --- /dev/null +++ b/examples/default-template/app/dev-routes.ts @@ -0,0 +1,5 @@ +import type { RouteConfig } from '@react-router/dev/routes'; + +// Kept separate so the dev-route-watch E2E covers route-config dependencies, +// not the direct reload-server watch on app/routes.ts. +export default [] satisfies RouteConfig; diff --git a/examples/default-template/app/routes.ts b/examples/default-template/app/routes.ts index 7ee17d8..463e452 100644 --- a/examples/default-template/app/routes.ts +++ b/examples/default-template/app/routes.ts @@ -5,6 +5,7 @@ import { prefix, route, } from '@react-router/dev/routes'; +import devRoutes from './dev-routes'; export default [ // Index route for the home page @@ -19,6 +20,8 @@ export default [ // Client loader/action example route('client-features', 'routes/client-features.tsx'), + ...devRoutes, + // Docs section with nested routes ...prefix('docs', [ layout('routes/docs/layout.tsx', [ diff --git a/examples/default-template/playwright.config.ts b/examples/default-template/playwright.config.ts index d3c3690..1029bdc 100644 --- a/examples/default-template/playwright.config.ts +++ b/examples/default-template/playwright.config.ts @@ -5,9 +5,9 @@ export default defineConfig({ // Maximum time one test can run for timeout: 30 * 1000, expect: { - timeout: 5000 + timeout: 5000, }, - // Keep this example serial because dev-route-watch mutates routes.ts and + // Keep this example serial because dev-route-watch mutates route config and // restarts the shared dev server. fullyParallel: false, workers: 1, diff --git a/examples/default-template/tests/e2e/dev-route-watch.test.ts b/examples/default-template/tests/e2e/dev-route-watch.test.ts index 32035c7..56dceef 100644 --- a/examples/default-template/tests/e2e/dev-route-watch.test.ts +++ b/examples/default-template/tests/e2e/dev-route-watch.test.ts @@ -1,5 +1,11 @@ import { expect, test, type Page } from '@playwright/test'; -import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { + existsSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -9,20 +15,28 @@ const restartMarkerPath = join( __dirname, '../../build/client/.react-router/route-watch' ); -const routesConfigPath = join(appDirectory, 'routes.ts'); +const devRoutesConfigPath = join(appDirectory, 'dev-routes.ts'); const addedRoutePath = join(appDirectory, 'routes/dev-added-route.tsx'); const addedRouteUrl = '/dev-added-route'; const addedRouteText = 'Route added while dev server is running'; const editedAddedRouteText = 'Route edited without dev server restart'; -const addedRouteConfigEntry = ` route('dev-added-route', 'routes/dev-added-route.tsx'),`; +const emptyDevRoutesConfig = `import type { RouteConfig } from '@react-router/dev/routes'; + +// Kept separate so the dev-route-watch E2E covers route-config dependencies, +// not the direct reload-server watch on app/routes.ts. +export default [] satisfies RouteConfig; +`; +const populatedDevRoutesConfig = `import { route, type RouteConfig } from '@react-router/dev/routes'; + +export default [ + route('dev-added-route', 'routes/dev-added-route.tsx'), +] satisfies RouteConfig; +`; const removeAddedRouteConfig = (): boolean => { - const routesConfig = readFileSync(routesConfigPath, 'utf8'); - if (routesConfig.includes(addedRouteConfigEntry)) { - writeFileSync( - routesConfigPath, - routesConfig.replace(`${addedRouteConfigEntry}\n\n`, '') - ); + const routesConfig = readFileSync(devRoutesConfigPath, 'utf8'); + if (routesConfig !== emptyDevRoutesConfig) { + writeFileSync(devRoutesConfigPath, emptyDevRoutesConfig); return true; } return false; @@ -36,10 +50,20 @@ const removeAddedRouteFile = (): boolean => { return false; }; -const readRestartMarker = (): string | null => - existsSync(restartMarkerPath) - ? readFileSync(restartMarkerPath, 'utf8') - : null; +const readRestartMarkerVersion = (): string | null => { + try { + if (!existsSync(restartMarkerPath)) { + return null; + } + const { mtimeNs } = statSync(restartMarkerPath, { bigint: true }); + return `${readFileSync(restartMarkerPath, 'utf8')}:${mtimeNs}`; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + throw error; + } +}; const expectRestartMarkerStable = async ( expectedMarker: string | null, @@ -49,7 +73,7 @@ const expectRestartMarkerStable = async ( await expect .poll( () => { - const marker = readRestartMarker(); + const marker = readRestartMarkerVersion(); if (marker !== expectedMarker) { return `changed:${marker ?? 'missing'}`; } @@ -60,11 +84,7 @@ const expectRestartMarkerStable = async ( .toBe('stable'); }; -const waitForRouteText = async ( - page: Page, - url: string, - text: string -) => { +const waitForRouteText = async (page: Page, url: string, text: string) => { await expect .poll( async () => { @@ -86,25 +106,37 @@ const waitForRouteText = async ( .toBe('ready'); }; +const waitForRouteToBeRemoved = async (page: Page, url: string) => { + await expect + .poll( + async () => { + try { + const response = await page.request.get(url, { timeout: 2000 }); + return response.status() === 404 ? 'removed' : response.status(); + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + }, + { timeout: 60000 } + ) + .toBe('removed'); +}; + test.describe('dev route watch', () => { test.setTimeout(90000); test.beforeEach(async ({ page }) => { if (removeAddedRouteConfig()) { - await waitForRouteText(page, '/', 'Welcome to React Router'); - } - if (removeAddedRouteFile()) { - await waitForRouteText(page, '/', 'Welcome to React Router'); + await waitForRouteToBeRemoved(page, addedRouteUrl); } + removeAddedRouteFile(); }); test.afterEach(async ({ page }) => { if (removeAddedRouteConfig()) { - await waitForRouteText(page, '/', 'Welcome to React Router'); - } - if (removeAddedRouteFile()) { - await waitForRouteText(page, '/', 'Welcome to React Router'); + await waitForRouteToBeRemoved(page, addedRouteUrl); } + removeAddedRouteFile(); }); test('serves a route added after the dev server starts without restarting on later edits', async ({ @@ -112,6 +144,7 @@ test.describe('dev route watch', () => { }) => { await page.goto('/'); await expect(page.locator('h1')).toContainText('Welcome to React Router'); + const restartMarkerBeforeAdd = readRestartMarkerVersion(); writeFileSync( addedRoutePath, @@ -121,22 +154,17 @@ test.describe('dev route watch', () => { ` ); - const routesConfig = readFileSync(routesConfigPath, 'utf8'); - writeFileSync( - routesConfigPath, - routesConfig.replace( - ' // Docs section with nested routes', - `${addedRouteConfigEntry}\n\n // Docs section with nested routes` - ) - ); + writeFileSync(devRoutesConfigPath, populatedDevRoutesConfig); await waitForRouteText(page, addedRouteUrl, addedRouteText); await page.goto(addedRouteUrl); await expect(page.locator('h1')).toHaveText(addedRouteText); - await expect.poll(readRestartMarker, { timeout: 10000 }).not.toBe(null); - const restartMarkerBefore = readRestartMarker(); + await expect + .poll(readRestartMarkerVersion, { timeout: 10000 }) + .not.toBe(restartMarkerBeforeAdd); + const restartMarkerBefore = readRestartMarkerVersion(); writeFileSync( addedRoutePath, `export default function DevAddedRoute() { diff --git a/rslib.config.ts b/rslib.config.ts index f00f09c..566bf25 100644 --- a/rslib.config.ts +++ b/rslib.config.ts @@ -5,16 +5,30 @@ import { } from '@rsbuild/config/rslib.config.js'; import { defineConfig } from '@rslib/core'; const config = defineConfig({ - source: { - entry: { - index: './src/index.ts', - 'parallel-route-transform-worker': - './src/parallel-route-transform-worker.ts', - 'templates/entry.server': './src/templates/entry.server.tsx', - 'templates/entry.client': './src/templates/entry.client.tsx', + lib: [ + { + ...esmConfig, + source: { + entry: { + index: './src/index.ts', + 'parallel-route-transform-worker': + './src/parallel-route-transform-worker.ts', + 'templates/entry.server': './src/templates/entry.server.tsx', + 'templates/entry.client': './src/templates/entry.client.tsx', + }, + }, }, - }, - lib: [esmConfig, cjsConfig], + { + ...cjsConfig, + source: { + entry: { + index: './src/index.ts', + 'templates/entry.server': './src/templates/entry.server.tsx', + 'templates/entry.client': './src/templates/entry.client.tsx', + }, + }, + }, + ], tools: { rspack: { externals: [ diff --git a/scripts/bench-builds.mjs b/scripts/bench-builds.mjs index 265d76b..74f7a04 100644 --- a/scripts/bench-builds.mjs +++ b/scripts/bench-builds.mjs @@ -88,6 +88,7 @@ const parseArgs = argv => { 'rspack-trace-output': { type: 'string' }, 'fail-fast': { type: 'boolean', default: false }, 'skip-root-build': { type: 'boolean', default: false }, + 'log-performance': { type: 'boolean', default: false }, }, }); @@ -98,7 +99,10 @@ const parseArgs = argv => { if (value === 'false' || value === '0') { return false; } - if (value === 'true' || value === '1' || value === 'auto') { + if (value === 'auto') { + return undefined; + } + if (value === 'true' || value === '1') { return true; } const maxWorkers = Number(value); @@ -123,6 +127,7 @@ const parseArgs = argv => { rspackTraceOutput: values['rspack-trace-output'] ?? null, failFast: values['fail-fast'], skipRootBuild: values['skip-root-build'], + logPerformance: values['log-performance'], }; if (!profiles[args.profile]) { @@ -155,7 +160,12 @@ const parseArgs = argv => { const hasGnuTime = async () => { try { await access('/usr/bin/time'); - return true; + const probe = await execa( + '/usr/bin/time', + ['-v', process.execPath, '-e', ''], + { reject: false } + ); + return probe.exitCode === 0; } catch { return false; } @@ -310,12 +320,13 @@ const renderMarkdown = result => { `- Iterations: ${result.iterations}`, `- Warmup: ${result.warmup}`, `- Parallel transforms: ${formatParallelTransforms(result.parallelTransforms)}`, + `- Plugin performance logging: ${String(result.logPerformance)}`, `- Rspack profile: ${result.rspackProfile ?? 'false'}`, ...(result.rspackTraceOutput ? [`- Rspack trace output: ${result.rspackTraceOutput}`] : []), '', - '| Benchmark | Routes | Variant | Median wall | Mean wall | p95 wall | Max RSS | Plugin reports |', + '| Benchmark | Routes | Variant | Median wall | Mean wall | p95 wall | Max RSS | Plugin reports (--log-performance) |', '|---|---:|---|---:|---:|---:|---:|---:|', ]; @@ -423,7 +434,7 @@ const writeOutputs = async (result, outputPaths) => { const formatParallelTransforms = parallelTransforms => { if (parallelTransforms === undefined) { - return 'default'; + return 'adaptive'; } if (!parallelTransforms) { return 'false'; @@ -536,6 +547,9 @@ const main = async () => { const pluginImportPath = pathToFileURL( path.join(rootDir, 'dist/index.js') ).href; + const pluginReactImportPath = + process.env.REACT_ROUTER_BENCHMARK_PLUGIN_REACT_IMPORT ?? + '@rsbuild/plugin-react'; const selectedBenchmarks = profiles[args.profile].filter(benchmark => args.filter ? benchmark.id.includes(args.filter) : true ); @@ -565,6 +579,7 @@ const main = async () => { variant: benchmark.variant, sourceMap: benchmark.sourceMap ?? false, pluginImportPath, + pluginReactImportPath, parallelTransforms: args.parallelTransforms, }); @@ -603,7 +618,9 @@ const main = async () => { cwd: fixtureRoot, env: { NODE_ENV: 'production', - REACT_ROUTER_BENCHMARK_LOG_PERFORMANCE: '1', + ...(args.logPerformance + ? { REACT_ROUTER_BENCHMARK_LOG_PERFORMANCE: '1' } + : {}), ...(args.rspackProfile ? { RSPACK_PROFILE: args.rspackProfile } : {}), ...(rspackTraceOutput ? { RSPACK_TRACE_OUTPUT: rspackTraceOutput } @@ -674,6 +691,8 @@ const main = async () => { profile: args.profile, iterations: args.iterations, warmup: args.warmup, + clean: args.clean, + logPerformance: args.logPerformance, parallelTransforms: args.parallelTransforms, rspackProfile: args.rspackProfile, rspackTraceOutput: args.rspackTraceOutput, diff --git a/scripts/benchmark/fixture.mjs b/scripts/benchmark/fixture.mjs index d3c94e3..5869b22 100644 --- a/scripts/benchmark/fixture.mjs +++ b/scripts/benchmark/fixture.mjs @@ -237,6 +237,7 @@ const createRsbuildConfig = ({ variant, sourceMap, pluginImportPath, + pluginReactImportPath, parallelTransforms, }) => { const ssr = variant !== 'spa'; @@ -248,7 +249,7 @@ const createRsbuildConfig = ({ return [ `import { defineConfig } from '@rsbuild/core';`, - `import { pluginReact } from '@rsbuild/plugin-react';`, + `import { pluginReact } from '${pluginReactImportPath}';`, `import { pluginReactRouter } from '${pluginImportPath}';`, '', 'export default defineConfig({', @@ -361,6 +362,7 @@ export async function generateSyntheticFixture({ variant, sourceMap = false, pluginImportPath = 'rsbuild-plugin-react-router', + pluginReactImportPath = '@rsbuild/plugin-react', fixture = 'default', parallelTransforms, }) { @@ -385,6 +387,7 @@ export async function generateSyntheticFixture({ variant, sourceMap, pluginImportPath, + pluginReactImportPath, parallelTransforms, }) ); diff --git a/src/export-utils.ts b/src/export-utils.ts index 7dac8e9..403eb0f 100644 --- a/src/export-utils.ts +++ b/src/export-utils.ts @@ -1,40 +1,12 @@ import { readFile, stat } from 'node:fs/promises'; -import { strip } from 'yuku-codegen'; import { langFromPath, parse } from 'yuku-parser'; import { setBoundedCacheEntry } from './bounded-cache.js'; -import { - detectRouteChunksIfEnabled, - type RouteChunkCache, - type RouteChunkConfig, - type RouteChunkInfo, -} from './route-chunks.js'; - -type TransformCacheEntry = { - source: string; - transformed: Promise; -}; type ExportInfo = { readonly exportNames: readonly string[]; readonly exportAllModules: readonly string[]; }; -type TransformedModule = ExportInfo & { - readonly code: string; -}; - -export type BundlerRouteAnalysis = TransformedModule & { - getRouteChunkInfo: ( - cache: RouteChunkCache | undefined, - config: RouteChunkConfig - ) => Promise; -}; - -type BundlerRouteAnalysisCacheEntry = { - source: string; - analysis: Promise; -}; - type RouteModuleAnalysis = { readonly code: string; readonly exports: readonly string[]; @@ -47,12 +19,7 @@ type RouteModuleAnalysisCacheEntry = { analysis: Promise; }; -const transformCache = new Map(); const exportInfoCache = new Map>(); -const bundlerRouteAnalysisCache = new Map< - string, - BundlerRouteAnalysisCacheEntry ->(); const routeModuleAnalysisCache = new Map< string, RouteModuleAnalysisCacheEntry @@ -71,9 +38,6 @@ const cachePromiseOnReject = ( throw error; }); -const getRouteChunkConfigCacheKey = (config: RouteChunkConfig) => - `${String(config.splitRouteModules ?? false)}\0${config.appDirectory}\0${config.rootRouteFile}`; - const parseProgram = (code: string, resourcePath?: string) => { const result = parse(code, { sourceType: 'module', @@ -217,126 +181,12 @@ const collectExportAllModules = (program: AnyNode): string[] => { return modules; }; -const getTransformedModule = async ( - code: string, - resourcePath: string -): Promise => { - const cached = transformCache.get(resourcePath); - if (cached?.source === code) { - return cached.transformed; - } - - let transformed: Promise; - transformed = cachePromiseOnReject( - (async () => { - const program = parseProgram(code, resourcePath); - const stripped = strip(program, { comments: 'some' }); - if (stripped.errors.length > 0) { - throw new Error(stripped.errors.map(error => error.message).join('\n')); - } - return { - code: stripped.code, - exportNames: collectProgramExportNames(program), - exportAllModules: collectExportAllModules(program), - }; - })(), - () => { - if (transformCache.get(resourcePath)?.transformed === transformed) { - transformCache.delete(resourcePath); - } - } - ); - - setBoundedCacheEntry( - transformCache, - resourcePath, - { - source: code, - transformed, - }, - MAX_EXPORT_UTILS_CACHE_ENTRIES - ); - return transformed; -}; - -export const transformToEsm = async ( - code: string, - resourcePath: string -): Promise => (await getTransformedModule(code, resourcePath)).code; - export const getExportNames = async ( code: string ): Promise => { return (await getExportNamesAndExportAll(code)).exportNames; }; -export const getBundlerRouteAnalysis = async ( - source: string, - resourcePath: string -): Promise => { - const cached = bundlerRouteAnalysisCache.get(resourcePath); - if (cached?.source === source) { - return cached.analysis; - } - - const analysis = (async () => { - const program = parseProgram(source, resourcePath); - const sourceInfo: TransformedModule = { - code: source, - exportNames: collectProgramExportNames(program), - exportAllModules: collectExportAllModules(program), - }; - const routeChunkInfoCache = new Map>(); - - return { - ...sourceInfo, - getRouteChunkInfo: ( - cache: RouteChunkCache | undefined, - config: RouteChunkConfig - ) => { - const cacheKey = getRouteChunkConfigCacheKey(config); - const cachedRouteChunkInfo = routeChunkInfoCache.get(cacheKey); - if (cachedRouteChunkInfo) { - return cachedRouteChunkInfo; - } - - let routeChunkInfo: Promise; - routeChunkInfo = cachePromiseOnReject( - detectRouteChunksIfEnabled(cache, config, resourcePath, source), - () => { - if (routeChunkInfoCache.get(cacheKey) === routeChunkInfo) { - routeChunkInfoCache.delete(cacheKey); - } - } - ); - - routeChunkInfoCache.set(cacheKey, routeChunkInfo); - return routeChunkInfo; - }, - }; - })(); - - let trackedAnalysis: Promise; - trackedAnalysis = cachePromiseOnReject(analysis, () => { - if ( - bundlerRouteAnalysisCache.get(resourcePath)?.analysis === trackedAnalysis - ) { - bundlerRouteAnalysisCache.delete(resourcePath); - } - }); - - setBoundedCacheEntry( - bundlerRouteAnalysisCache, - resourcePath, - { - source, - analysis: trackedAnalysis, - }, - MAX_EXPORT_UTILS_CACHE_ENTRIES - ); - return trackedAnalysis; -}; - export const getExportNamesAndExportAll = async ( code: string ): Promise => { @@ -380,11 +230,11 @@ export const getRouteModuleAnalysis = async ( const analysis = (async () => { const source = await readFile(resourcePath, 'utf8'); - const transformed = await getTransformedModule(source, resourcePath); + const program = parseProgram(source, resourcePath); return { - code: transformed.code, - exports: transformed.exportNames, - exportAllModules: transformed.exportAllModules, + code: source, + exports: collectProgramExportNames(program), + exportAllModules: collectExportAllModules(program), }; })(); diff --git a/src/index.ts b/src/index.ts index fcd7560..b4f6c9e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,6 +4,7 @@ import { pathToFileURL } from 'node:url'; import fsExtra from 'fs-extra'; import type { Config } from './react-router-config.js'; import type { RouteConfigEntry } from '@react-router/dev/routes'; +import type { ResultPromise } from 'execa'; import { rspack, type RsbuildEntryDescription, @@ -14,7 +15,11 @@ import { createJiti } from 'jiti'; import jsesc from 'jsesc'; import { dirname, relative, resolve } from 'pathe'; -import { BUILD_CLIENT_ROUTE_QUERY_STRING, PLUGIN_NAME } from './constants.js'; +import { + BUILD_CLIENT_ROUTE_QUERY_STRING, + JS_EXTENSIONS, + PLUGIN_NAME, +} from './constants.js'; import { createDevServerMiddleware } from './dev-server.js'; import { generateWithProps, @@ -38,10 +43,11 @@ import { } from './react-router-config.js'; import { getReactRouterManifestForDev, - getRouteManifestModuleExports, + generateReactRouterManifestForDev, configRoutesToRouteManifest, createReactRouterManifestStats, type ReactRouterManifestStats, + type RouteManifestModuleExports, } from './manifest.js'; import { createModifyBrowserManifestPlugin } from './modify-browser-manifest.js'; import { createRequestHandler, matchRoutes } from 'react-router'; @@ -52,11 +58,13 @@ import { type RouteChunkCache, type RouteChunkConfig, } from './route-chunks.js'; -import { createRouteTransformExecutor } from './parallel-route-transforms.js'; +import { + createRouteTransformExecutor, + shouldParallelizeRouteTransforms, +} from './parallel-route-transforms.js'; import { createRouteTopologyWatcher, createRouteManifestSnapshot, - emitRouteRestartMarkerAsset, ensureDevRestartMarker, getRouteRestartMarkerPath, mergeWatchFiles, @@ -67,7 +75,10 @@ import { getBuildManifest, getRoutesByServerBundleId, } from './build-manifest.js'; -import { warnOnClientSourceMaps } from './warnings/warn-on-client-source-maps.js'; +import { + isSourceMapEnabled, + warnOnClientSourceMaps, +} from './warnings/warn-on-client-source-maps.js'; import { validatePluginOrderFromConfig } from './validation/validate-plugin-order.js'; import { getSsrExternals } from './ssr-externals.js'; import { @@ -167,11 +178,16 @@ export const pluginReactRouter = ( warnOnClientSourceMaps(normalized, msg => api.logger.warn(msg), 'web'); }); + let typegenProcess: ResultPromise | undefined; + // Run typegen on build/dev api.onBeforeStartDevServer(async () => { + if (typegenProcess) { + return; + } const { execa } = await import('execa'); // Run typegen in background (non-blocking) for watch mode - const child = execa( + const process = execa( 'npx', ['--yes', 'react-router', 'typegen', '--watch'], { @@ -180,10 +196,27 @@ export const pluginReactRouter = ( cleanup: true, } ); + typegenProcess = process; // Don't await - let it run in the background - child.catch(() => { - // Silently ignore errors when the process is killed on server shutdown - }); + process + .catch(() => { + // Silently ignore errors when the process is killed on server shutdown + }) + .finally(() => { + if (typegenProcess === process) { + typegenProcess = undefined; + } + }); + }); + + api.onCloseDevServer(async () => { + const process = typegenProcess; + typegenProcess = undefined; + if (!process) { + return; + } + process.kill('SIGTERM'); + await process.catch(() => undefined); }); api.onBeforeBuild(async () => { @@ -201,6 +234,11 @@ export const pluginReactRouter = ( // Read the react-router.config file first (supports .ts, .js, .mjs, etc.) const configPath = findEntryFile(resolve('react-router.config')); const configExists = existsSync(configPath); + const configWatchPaths = configExists + ? configPath + : JS_EXTENSIONS.map(extension => + resolve(`react-router.config${extension}`) + ); let reactRouterUserConfig: Config = {}; if (!configExists) { console.warn( @@ -369,14 +407,16 @@ export const pluginReactRouter = ( ? entryServerPath : templateServerPath; - const rootRoutePath = findEntryFile(resolve(appDirectory, 'root')); + const getRootRoutePath = () => findEntryFile(resolve(appDirectory, 'root')); + const rootRoutePath = getRootRoutePath(); // React Router's server build expects route files relative to `appDirectory` // so it can resolve them correctly during compilation. const rootRouteFile = relative(appDirectory, rootRoutePath); const getWatchedRouteTopology = async (): Promise> => { const latestRouteConfig = await loadRouteConfig(); + const latestRootRouteFile = relative(appDirectory, getRootRoutePath()); const latestRoutes = { - root: { path: '', id: 'root', file: rootRouteFile }, + root: { path: '', id: 'root', file: latestRootRouteFile }, ...configRoutesToRouteManifest(appDirectory, latestRouteConfig), }; return createRouteManifestSnapshot(latestRoutes); @@ -419,7 +459,9 @@ export const pluginReactRouter = ( }; const routeChunkCache: RouteChunkCache = new Map(); const routeTransformExecutor = createRouteTransformExecutor({ - parallelTransforms: pluginOptions.parallelTransforms, + parallelTransforms: + pluginOptions.parallelTransforms ?? + shouldParallelizeRouteTransforms(routeCount), routeChunkCache, splitRouteModules: Boolean(splitRouteModules), }); @@ -434,6 +476,10 @@ export const pluginReactRouter = ( const watchDirectory = resolve(appDirectory); const routeRestartMarkerPath = getRouteRestartMarkerPath(outputClientPath); const routeWatchFiles: WatchFileConfig[] = [ + { + paths: configWatchPaths, + type: 'reload-server', + }, { paths: routesPath, type: 'reload-server', @@ -443,14 +489,16 @@ export const pluginReactRouter = ( type: 'reload-server', }, ]; - let closeRouteTopologyWatcher: (() => void) | undefined; + let closeRouteTopologyWatcher: (() => Promise) | undefined; api.onBeforeStartDevServer(async () => { await ensureDevRestartMarker(routeRestartMarkerPath); closeRouteTopologyWatcher = await createRouteTopologyWatcher({ watchDirectory, getRouteTopology: getWatchedRouteTopology, + initialRouteTopology: createRouteManifestSnapshot(routes), restartMarkerPath: routeRestartMarkerPath, + onRouteTopologyChange: pluginOptions.onRouteTopologyChange, onError: error => { api.logger.warn( `[${PLUGIN_NAME}] Failed to watch route topology changes: ${error}` @@ -459,8 +507,8 @@ export const pluginReactRouter = ( }); }); - api.onCloseDevServer(() => { - closeRouteTopologyWatcher?.(); + api.onCloseDevServer(async () => { + await closeRouteTopologyWatcher?.(); closeRouteTopologyWatcher = undefined; }); api.onCloseBuild(async () => { @@ -474,6 +522,7 @@ export const pluginReactRouter = ( ReturnType >; let latestBrowserManifest: ReactRouterManifest | null = null; + let latestBrowserManifestModuleExports: RouteManifestModuleExports = {}; let latestServerManifest: ReactRouterManifest | null = null; const latestServerManifestsByBundleId: Record = {}; @@ -822,6 +871,7 @@ export const pluginReactRouter = ( const validateSsrFalsePrerenderExports = async ( manifest: Awaited>, + routeExports: RouteManifestModuleExports, prerenderList: string[] ) => { if (prerenderList.length === 0) { @@ -845,8 +895,6 @@ export const pluginReactRouter = ( ); } - const routeExports = getRouteManifestModuleExports(manifest); - const errors: string[] = []; for (const [routeId, route] of Object.entries(manifest.routes)) { const exports = routeExports[routeId] ?? []; @@ -952,17 +1000,24 @@ export const pluginReactRouter = ( if (isPrerenderEnabled) { if (!ssr) { - const manifest = - latestBrowserManifest ?? - (await getReactRouterManifestForDev( - routes, - pluginOptions, - clientStats, - appDirectory, - assetPrefix, - routeChunkOptions - )); - await validateSsrFalsePrerenderExports(manifest, prerenderPaths); + const generated = latestBrowserManifest + ? { + manifest: latestBrowserManifest, + moduleExportsByRouteId: latestBrowserManifestModuleExports, + } + : await generateReactRouterManifestForDev( + routes, + pluginOptions, + clientStats, + appDirectory, + assetPrefix, + routeChunkOptions + ); + await validateSsrFalsePrerenderExports( + generated.manifest, + generated.moduleExportsByRouteId, + prerenderPaths + ); } const routeTree = createPrerenderRoutes(routes); @@ -1357,13 +1412,15 @@ export const pluginReactRouter = ( { future, manifestChunkNames, - onManifest: (manifest, sri) => { + onManifest: (manifest, sri, moduleExportsByRouteId) => { performanceProfiler.recordSync( 'web', 'manifest:stage', 'virtual/react-router/browser-manifest', () => { latestBrowserManifest = manifest; + latestBrowserManifestModuleExports = + moduleExportsByRouteId; const baseServerManifest = { ...manifest, sri, @@ -1403,19 +1460,6 @@ export const pluginReactRouter = ( } ); - if (isBuild) { - api.processAssets( - { stage: 'additional', targets: ['web'] }, - ({ sources, compilation }) => { - emitRouteRestartMarkerAsset({ - restartMarkerPath: routeRestartMarkerPath, - sources, - compilation, - }); - } - ); - } - api.processAssets( { stage: 'additional', targets: ['node'] }, ({ sources, compilation }) => { @@ -1602,6 +1646,9 @@ export const pluginReactRouter = ( resource: args.resource, resourcePath: args.resourcePath, environmentName: args.environment.name, + sourceMaps: isSourceMapEnabled( + args.environment.config.output.sourceMap + ), ssr, isBuild, isSpaMode, diff --git a/src/manifest.ts b/src/manifest.ts index 72463c9..808a234 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -146,14 +146,10 @@ export const createReactRouterManifestStats = ( export type RouteManifestModuleExports = Record; -const routeManifestModuleExports = new WeakMap< - ReactRouterManifestForDev, - RouteManifestModuleExports ->(); - -export const getRouteManifestModuleExports = ( - manifest: ReactRouterManifestForDev -): RouteManifestModuleExports => routeManifestModuleExports.get(manifest) ?? {}; +export type ReactRouterManifestGenerationResult = { + manifest: ReactRouterManifestForDev; + moduleExportsByRouteId: RouteManifestModuleExports; +}; const DEFAULT_MANIFEST_DIR = 'static/js'; @@ -216,15 +212,14 @@ export const getReactRouterManifestChunkNames = ( return chunkNames; }; -export async function getReactRouterManifestForDev( +export async function generateReactRouterManifestForDev( routes: Record, - //@ts-ignore - options: PluginOptions, + _options: PluginOptions, clientStats: ReactRouterManifestStats | undefined, context: string, assetPrefix = '/', routeChunkOptions?: RouteChunkManifestOptions -): Promise { +): Promise { const result: Record = {}; const splitRouteModules = routeChunkOptions?.splitRouteModules ?? false; const enforceSplitRouteModules = splitRouteModules === 'enforce'; @@ -395,6 +390,14 @@ export async function getReactRouterManifestForDev( routes: result, }; - routeManifestModuleExports.set(manifest, routeModuleExportsByRouteId); - return manifest; + return { + manifest, + moduleExportsByRouteId: routeModuleExportsByRouteId, + }; +} + +export async function getReactRouterManifestForDev( + ...args: Parameters +): Promise { + return (await generateReactRouterManifestForDev(...args)).manifest; } diff --git a/src/modify-browser-manifest.ts b/src/modify-browser-manifest.ts index b172eac..4bcb5a7 100644 --- a/src/modify-browser-manifest.ts +++ b/src/modify-browser-manifest.ts @@ -4,6 +4,7 @@ import { rspack } from '@rsbuild/core'; import type { Rspack } from '@rsbuild/core'; import { createReactRouterManifestStats, + generateReactRouterManifestForDev, getReactRouterManifestChunkNames, getReactRouterManifestForDev, getReactRouterManifestPath, @@ -29,7 +30,10 @@ export function createModifyBrowserManifestPlugin( manifestChunkNames?: ReadonlySet; onManifest?: ( manifest: Awaited>, - sri: Record | undefined + sri: Record | undefined, + moduleExportsByRouteId: Awaited< + ReturnType + >['moduleExportsByRouteId'] ) => void; } ) { @@ -42,21 +46,22 @@ export function createModifyBrowserManifestPlugin( return { apply(compiler: Rspack.Compiler): void { - compiler.hooks.emit.tapAsync( + compiler.hooks.emit.tapPromise( 'ModifyBrowserManifest', - async (compilation: Rspack.Compilation, callback) => { + async (compilation: Rspack.Compilation) => { const stats = createReactRouterManifestStats( compilation, manifestChunkNames ); - const manifest = await getReactRouterManifestForDev( - routes, - pluginOptions, - stats, - appDirectory, - assetPrefix, - routeChunkOptions - ); + const { manifest, moduleExportsByRouteId } = + await generateReactRouterManifestForDev( + routes, + pluginOptions, + stats, + appDirectory, + assetPrefix, + routeChunkOptions + ); const virtualManifestPath = 'static/js/virtual/react-router/browser-manifest.js'; @@ -136,8 +141,7 @@ export function createModifyBrowserManifestPlugin( } } - options?.onManifest?.(manifest, sri); - callback(); + options?.onManifest?.(manifest, sri, moduleExportsByRouteId); } ); }, diff --git a/src/parallel-route-transform-protocol.ts b/src/parallel-route-transform-protocol.ts new file mode 100644 index 0000000..2492922 --- /dev/null +++ b/src/parallel-route-transform-protocol.ts @@ -0,0 +1,35 @@ +import type { + RouteTransformResult, + RouteTransformTask, +} from './route-transform-tasks.js'; + +type WithoutRequiredSource = Task extends RouteTransformTask + ? Omit & { code?: string } + : never; + +export type CachedRouteTransformTask = + WithoutRequiredSource; + +export type WorkerRequest = { + id: number; + task: RouteTransformTask | CachedRouteTransformTask; + sourceCacheKey?: string; +}; + +export type WorkerErrorPayload = { + name?: string; + message: string; + stack?: string; +}; + +export type WorkerResponse = + | { + id: number; + ok: true; + result: RouteTransformResult; + } + | { + id: number; + ok: false; + error: WorkerErrorPayload; + }; diff --git a/src/parallel-route-transform-worker.ts b/src/parallel-route-transform-worker.ts index e9c0ad3..36e85e2 100644 --- a/src/parallel-route-transform-worker.ts +++ b/src/parallel-route-transform-worker.ts @@ -2,37 +2,13 @@ import { parentPort } from 'node:worker_threads'; import { setBoundedCacheEntry } from './bounded-cache.js'; import { executeRouteTransformTask, - type RouteTransformResult, type RouteTransformTask, } from './route-transform-tasks.js'; - -type CachedRouteTransformTask = Omit & { - code?: string; -}; - -type WorkerRequest = { - id: number; - task: RouteTransformTask | CachedRouteTransformTask; - sourceCacheKey?: string; -}; - -type WorkerErrorPayload = { - name?: string; - message: string; - stack?: string; -}; - -type WorkerResponse = - | { - id: number; - ok: true; - result: RouteTransformResult; - } - | { - id: number; - ok: false; - error: WorkerErrorPayload; - }; +import type { + WorkerErrorPayload, + WorkerRequest, + WorkerResponse, +} from './parallel-route-transform-protocol.js'; const serializeError = (error: unknown): WorkerErrorPayload => { if (error instanceof Error) { diff --git a/src/parallel-route-transforms.ts b/src/parallel-route-transforms.ts index 6eddf6d..c33a8f7 100644 --- a/src/parallel-route-transforms.ts +++ b/src/parallel-route-transforms.ts @@ -1,6 +1,5 @@ import { Worker } from 'node:worker_threads'; import { setBoundedCacheEntry } from './bounded-cache.js'; -import { SERVER_ONLY_ROUTE_EXPORTS } from './constants.js'; import { getDefaultConcurrency } from './concurrency.js'; import { executeRouteTransformTask, @@ -9,6 +8,11 @@ import { type RouteTransformTaskOptions, } from './route-transform-tasks.js'; import type { PluginOptions } from './types.js'; +import type { + WorkerErrorPayload, + WorkerRequest, + WorkerResponse, +} from './parallel-route-transform-protocol.js'; export type ParallelTransformsConfig = NonNullable extends infer Config @@ -25,32 +29,6 @@ export type RouteTransformExecutor = { close: () => Promise; }; -type WorkerResponse = - | { - id: number; - ok: true; - result: RouteTransformResult; - } - | { - id: number; - ok: false; - error: WorkerErrorPayload; - }; - -type WorkerRequest = { - id: number; - task: - | RouteTransformTask - | (Omit & { code?: string }); - sourceCacheKey?: string; -}; - -type WorkerErrorPayload = { - name?: string; - message: string; - stack?: string; -}; - type PendingTask = { resolve: (result: RouteTransformResult) => void; reject: (error: Error) => void; @@ -63,11 +41,6 @@ type WorkerState = { startupError?: WorkerStartupError; }; -type RouteModuleResultCacheEntry = { - source: string; - result: Promise; -}; - class WorkerStartupError extends Error { constructor(message: string) { super(message); @@ -76,10 +49,15 @@ class WorkerStartupError extends Error { } const MAX_WORKER_SOURCE_CACHE_ENTRIES = 2048; -const MAX_ROUTE_MODULE_RESULT_CACHE_ENTRIES = 2048; +const AUTO_PARALLEL_ROUTE_THRESHOLD = 256; +const DEFAULT_MAX_WORKERS = 8; +const MAX_CONFIGURED_WORKERS = 32; export const getDefaultWorkerCount = (cpuCount?: number): number => - getDefaultConcurrency(cpuCount); + Math.min(DEFAULT_MAX_WORKERS, getDefaultConcurrency(cpuCount)); + +export const shouldParallelizeRouteTransforms = (routeCount: number): boolean => + routeCount >= AUTO_PARALLEL_ROUTE_THRESHOLD; const getConfiguredWorkerCount = ( parallelTransforms: ParallelTransformsConfig @@ -92,12 +70,17 @@ const getConfiguredWorkerCount = ( if (configured === undefined) { return getDefaultWorkerCount(); } - if (!Number.isFinite(configured) || configured < 1) { + if (!Number.isInteger(configured) || configured < 1) { throw new Error( - '[react-router] parallelTransforms.maxWorkers must be at least 1.' + '[react-router] parallelTransforms.maxWorkers must be a positive integer.' ); } - return Math.floor(configured); + if (configured > MAX_CONFIGURED_WORKERS) { + throw new Error( + `[react-router] parallelTransforms.maxWorkers must not exceed ${MAX_CONFIGURED_WORKERS}.` + ); + } + return configured; }; const hashString = (value: string): number => { @@ -123,31 +106,33 @@ const createWorkerUrl = (): URL => const isWorkerStartupError = (error: unknown): error is WorkerStartupError => error instanceof WorkerStartupError; -const canShareRouteModuleBuildResult = (task: RouteTransformTask): boolean => - task.kind === 'routeModule' && - task.isBuild && - task.ssr && - !task.isSpaMode && - !SERVER_ONLY_ROUTE_EXPORTS.some(exportName => task.code.includes(exportName)); - class ParallelRouteTransformExecutor implements RouteTransformExecutor { #closed = false; + #closePromise: Promise | undefined; + #workersDisabled = false; #nextId = 1; #nextRouteModuleWorkerIndex = 0; #nextSplitRouteAnalysisWorkerIndex = 0; - #routeModuleResultCache = new Map(); #splitRouteAnalysisWorkers = new Map(); #workers: WorkerState[]; constructor( workerCount: number, private readonly options: RouteTransformTaskOptions, - private readonly balanceRouteModuleTransforms: boolean, - private readonly shareRouteModuleBuildResults: boolean + private readonly balanceRouteModuleTransforms: boolean ) { - this.#workers = Array.from({ length: workerCount }, () => - this.#createWorkerState() - ); + this.#workers = []; + try { + for (let index = 0; index < workerCount; index += 1) { + this.#workers.push(this.#createWorkerState()); + } + } catch (error) { + for (const state of this.#workers) { + void state.worker.terminate(); + } + this.#workers = []; + throw error; + } } async run(task: RouteTransformTask): Promise { @@ -155,13 +140,6 @@ class ParallelRouteTransformExecutor implements RouteTransformExecutor { return executeRouteTransformTask(task, this.options); } - if ( - this.shareRouteModuleBuildResults && - canShareRouteModuleBuildResult(task) - ) { - return this.#runCachedRouteModuleBuildTask(task); - } - try { return await this.#runInWorker(task); } catch (error) { @@ -172,14 +150,14 @@ class ParallelRouteTransformExecutor implements RouteTransformExecutor { } } - async close(): Promise { - if (this.#closed) { - return; + close(): Promise { + if (this.#closePromise) { + return this.#closePromise; } this.#closed = true; const workers = this.#workers; this.#workers = []; - await Promise.all( + this.#closePromise = Promise.all( workers.map(async state => { for (const pending of state.pending.values()) { pending.reject(new Error('Route transform worker closed.')); @@ -187,7 +165,25 @@ class ParallelRouteTransformExecutor implements RouteTransformExecutor { state.pending.clear(); await state.worker.terminate(); }) - ); + ).then(() => undefined); + return this.#closePromise; + } + + #disableWorkers(error: WorkerStartupError): void { + if (this.#workersDisabled || this.#closed) { + return; + } + this.#workersDisabled = true; + const workers = this.#workers; + this.#workers = []; + for (const state of workers) { + state.startupError = error; + for (const pending of state.pending.values()) { + pending.reject(error); + } + state.pending.clear(); + void state.worker.terminate(); + } } #createWorkerState(): WorkerState { @@ -214,60 +210,22 @@ class ParallelRouteTransformExecutor implements RouteTransformExecutor { worker.on('error', (error: Error) => { const startupError = new WorkerStartupError(error.message); startupError.stack = error.stack; - state.startupError = startupError; - for (const pending of state.pending.values()) { - pending.reject(startupError); - } - state.pending.clear(); + this.#disableWorkers(startupError); }); worker.on('exit', code => { - if (this.#closed || code === 0) { + if (this.#closed || this.#workersDisabled) { return; } const startupError = new WorkerStartupError( `Route transform worker exited with code ${code}.` ); - state.startupError = startupError; - for (const pending of state.pending.values()) { - pending.reject(startupError); - } - state.pending.clear(); + this.#disableWorkers(startupError); }); return state; } - #runCachedRouteModuleBuildTask( - task: RouteTransformTask - ): Promise { - const cacheKey = task.resourcePath; - const cached = this.#routeModuleResultCache.get(cacheKey); - if (cached?.source === task.code) { - return cached.result; - } - - const result = this.#runInWorker(task).catch(error => { - if (this.#routeModuleResultCache.get(cacheKey)?.result === result) { - this.#routeModuleResultCache.delete(cacheKey); - } - if (isWorkerStartupError(error)) { - return executeRouteTransformTask(task, this.options); - } - throw error; - }); - setBoundedCacheEntry( - this.#routeModuleResultCache, - cacheKey, - { - source: task.code, - result, - }, - MAX_ROUTE_MODULE_RESULT_CACHE_ENTRIES - ); - return result; - } - #runInWorker(task: RouteTransformTask): Promise { const workerIndex = this.#getWorkerIndex(task); const state = this.#workers[workerIndex]; @@ -287,11 +245,19 @@ class ParallelRouteTransformExecutor implements RouteTransformExecutor { ); return new Promise((resolve, reject) => { state.pending.set(id, { resolve, reject }); - state.worker.postMessage({ - id, - task: requestTask, - sourceCacheKey, - } satisfies WorkerRequest); + try { + state.worker.postMessage({ + id, + task: requestTask, + sourceCacheKey, + } satisfies WorkerRequest); + } catch (error) { + state.pending.delete(id); + // The worker may not have received the source update. Force the next + // request for this module to send its full source again. + state.sourceCache.delete(sourceCacheKey); + reject(error instanceof Error ? error : new Error(String(error))); + } }); } @@ -353,7 +319,7 @@ export const createRouteTransformExecutor = ({ splitRouteModules, }: RouteTransformExecutorOptions = {}): RouteTransformExecutor => { const options = { routeChunkCache }; - const effectiveParallelTransforms = parallelTransforms ?? true; + const effectiveParallelTransforms = parallelTransforms ?? false; if (!effectiveParallelTransforms) { return { run: task => executeRouteTransformTask(task, options), @@ -372,7 +338,6 @@ export const createRouteTransformExecutor = ({ return new ParallelRouteTransformExecutor( workerCount, options, - Boolean(splitRouteModules), Boolean(splitRouteModules) ); }; diff --git a/src/performance.ts b/src/performance.ts index fb9adf6..1dedbd2 100644 --- a/src/performance.ts +++ b/src/performance.ts @@ -180,12 +180,24 @@ export const createReactRouterPerformanceProfiler = ({ return callback().then( result => { const end = performance.now(); - recordDuration(resolvedEnvironment, operation, resource, start, end); + recordDuration( + resolvedEnvironment, + operation, + resource, + start, + end + ); return result; }, error => { const end = performance.now(); - recordDuration(resolvedEnvironment, operation, resource, start, end); + recordDuration( + resolvedEnvironment, + operation, + resource, + start, + end + ); throw error; } ); diff --git a/src/plugin-utils.ts b/src/plugin-utils.ts index 1ba9166..5156198 100644 --- a/src/plugin-utils.ts +++ b/src/plugin-utils.ts @@ -1,118 +1,6 @@ import { normalize } from 'pathe'; import { existsSync } from 'node:fs'; -import { walk, type ParseResult } from 'yuku-parser'; -import { - NAMED_COMPONENT_EXPORTS, - NAMED_COMPONENT_EXPORTS_SET, - JS_EXTENSIONS, -} from './constants.js'; - -type AnyNode = Record; - -const getProgram = (ast: ParseResult | AnyNode): AnyNode => - (ast as ParseResult).program ?? ast; - -export function validateDestructuredExports( - id: AnyNode, - exportsToRemove: readonly string[] -): void { - if (id.type === 'Identifier') { - if (exportsToRemove.includes(id.name)) { - throw invalidDestructureError(id.name); - } - return; - } - - if (id.type === 'AssignmentPattern') { - validateDestructuredExports(id.left, exportsToRemove); - return; - } - - if (id.type === 'ArrayPattern') { - for (const element of id.elements ?? []) { - if (!element) { - continue; - } - - if (element.type === 'AssignmentPattern') { - validateDestructuredExports(element, exportsToRemove); - continue; - } - - if ( - element.type === 'Identifier' && - exportsToRemove.includes(element.name) - ) { - throw invalidDestructureError(element.name); - } - - if ( - element.type === 'RestElement' && - element.argument.type === 'Identifier' && - exportsToRemove.includes(element.argument.name) - ) { - throw invalidDestructureError(element.argument.name); - } - - if (element.type === 'ArrayPattern' || element.type === 'ObjectPattern') { - validateDestructuredExports(element, exportsToRemove); - } - } - } - - if (id.type === 'ObjectPattern') { - for (const property of id.properties ?? []) { - if (!property) { - continue; - } - - if (property.type === 'Property') { - if ( - property.value.type === 'Identifier' && - exportsToRemove.includes(property.value.name) - ) { - throw invalidDestructureError(property.value.name); - } - - if ( - property.value.type === 'AssignmentPattern' || - property.value.type === 'ArrayPattern' || - property.value.type === 'ObjectPattern' - ) { - validateDestructuredExports(property.value, exportsToRemove); - } - } - - if ( - property.type === 'RestElement' && - property.argument.type === 'Identifier' && - exportsToRemove.includes(property.argument.name) - ) { - throw invalidDestructureError(property.argument.name); - } - } - } -} - -export function invalidDestructureError(name: string): Error { - return new Error(`Cannot remove destructured export "${name}"`); -} - -export function toFunctionExpression(decl: AnyNode): AnyNode { - return { - ...decl, - type: 'FunctionExpression', - declare: undefined, - }; -} - -export function toClassExpression(decl: AnyNode): AnyNode { - return { - ...decl, - type: 'ClassExpression', - declare: undefined, - }; -} +import { JS_EXTENSIONS } from './constants.js'; export function combineURLs(baseURL: string, relativeURL: string): string { return relativeURL @@ -185,882 +73,10 @@ export function generateWithProps() { `; } -const removeFromArray = (array: T[], value: T): void => { - const index = array.indexOf(value); - if (index >= 0) { - array.splice(index, 1); - } -}; - -const getPatternIdentifierNames = ( - pattern: AnyNode | null | undefined, - names = new Set() -): Set => { - if (!pattern) { - return names; - } - if (pattern.type === 'Identifier') { - names.add(pattern.name); - return names; - } - if (pattern.type === 'RestElement') { - return getPatternIdentifierNames(pattern.argument, names); - } - if (pattern.type === 'AssignmentPattern') { - return getPatternIdentifierNames(pattern.left, names); - } - if (pattern.type === 'ArrayPattern') { - for (const element of pattern.elements ?? []) { - getPatternIdentifierNames(element, names); - } - return names; - } - if (pattern.type === 'ObjectPattern') { - for (const property of pattern.properties ?? []) { - if (property.type === 'RestElement') { - getPatternIdentifierNames(property.argument, names); - } else { - getPatternIdentifierNames(property.value, names); - } - } - } - return names; -}; - -const getDeclaredNames = (node: AnyNode): Set => { - const names = new Set(); - if (node.type === 'VariableDeclaration') { - for (const declarator of node.declarations ?? []) { - getPatternIdentifierNames(declarator.id, names); - } - } else if ( - (node.type === 'FunctionDeclaration' || node.type === 'ClassDeclaration') && - node.id?.name - ) { - names.add(node.id.name); - } else if (node.type === 'ImportDeclaration') { - for (const specifier of node.specifiers ?? []) { - if (specifier.local?.name) { - names.add(specifier.local.name); - } - } - } - return names; -}; - -const isIdentifierDeclaration = (node: AnyNode, parent: AnyNode | null) => { - if (!parent || node.type !== 'Identifier') { - return false; - } - if ( - (parent.type === 'FunctionDeclaration' || - parent.type === 'FunctionExpression' || - parent.type === 'ClassDeclaration' || - parent.type === 'ClassExpression') && - parent.id === node - ) { - return true; - } - if (parent.type === 'VariableDeclarator') { - return getPatternIdentifierNames(parent.id).has(node.name); - } - if ( - (parent.type === 'ImportSpecifier' || - parent.type === 'ImportDefaultSpecifier' || - parent.type === 'ImportNamespaceSpecifier') && - parent.local === node - ) { - return true; - } - if ( - (parent.type === 'FunctionDeclaration' || - parent.type === 'FunctionExpression' || - parent.type === 'ArrowFunctionExpression') && - (parent.params ?? []).some((param: AnyNode) => - getPatternIdentifierNames(param).has(node.name) - ) - ) { - return true; - } - return false; -}; - -const isNonReferenceIdentifier = (node: AnyNode, parent: AnyNode | null) => { - if (!parent || node.type !== 'Identifier') { - return false; - } - if (isIdentifierDeclaration(node, parent)) { - return true; - } - if ( - parent.type === 'MemberExpression' && - parent.property === node && - !parent.computed - ) { - return true; - } - if ( - parent.type === 'Property' && - parent.key === node && - !parent.computed && - !parent.shorthand - ) { - return true; - } - if ( - parent.type === 'MethodDefinition' && - parent.key === node && - !parent.computed - ) { - return true; - } - if ( - parent.type === 'ExportSpecifier' || - parent.type === 'ExportDefaultSpecifier' || - parent.type === 'ExportNamespaceSpecifier' - ) { - return true; - } - if (parent.type === 'ImportSpecifier' && parent.imported === node) { - return true; - } - if ( - parent.type === 'LabeledStatement' || - parent.type === 'BreakStatement' || - parent.type === 'ContinueStatement' - ) { - return true; - } - return false; -}; - -const isUppercaseName = (name: string): boolean => /^[A-Z]/.test(name); - -const collectReferencedNames = (node: AnyNode): Set => { - const referenced = new Set(); - walk(node as any, { - Identifier(node: AnyNode, ctx: any) { - const parent = ctx.parent as AnyNode | null; - if (!isNonReferenceIdentifier(node, parent)) { - referenced.add(node.name); - } - }, - JSXIdentifier(node: AnyNode, ctx: any) { - const parent = ctx.parent as AnyNode | null; - if (!parent || !isUppercaseName(node.name)) { - return; - } - if ( - (parent.type === 'JSXOpeningElement' || - parent.type === 'JSXClosingElement') && - parent.name === node - ) { - referenced.add(node.name); - return; - } - if (parent.type === 'JSXMemberExpression' && parent.object === node) { - referenced.add(node.name); - } - }, - ExportSpecifier(node: AnyNode, ctx: any) { - const declaration = ctx.parent as AnyNode | null; - if ( - !declaration?.source && - declaration?.exportKind !== 'type' && - node.local?.name && - node.exportKind !== 'type' - ) { - referenced.add(node.local.name); - } - }, - }); - return referenced; -}; - -const getExportedName = (specifier: AnyNode): string | null => { - const exported = specifier.exported; - if (!exported) { - return null; - } - if (exported.type === 'Identifier') { - return exported.name; - } - if (exported.type === 'Literal') { - return String(exported.value); - } - return null; -}; - -type TopLevelDeclaration = { - referencedNames: Set; -}; - -type TopLevelDeclarationGraph = { - declarationsByNode: Map; - declarationsByName: Map>; -}; - -const createTopLevelDeclarationGraph = ( - program: AnyNode -): TopLevelDeclarationGraph => { - const declarationsByNode = new Map(); - const declarationsByName = new Map>(); - - const registerDeclaration = ( - node: AnyNode, - declarationNode: AnyNode, - declaredNames: Set - ) => { - const declaration: TopLevelDeclaration = { - referencedNames: collectReferencedNames(declarationNode), - }; - declarationsByNode.set(node, declaration); - for (const name of declaredNames) { - const namedDeclarations = declarationsByName.get(name) ?? new Set(); - namedDeclarations.add(declaration); - declarationsByName.set(name, namedDeclarations); - } - }; - - for (const statement of program.body ?? []) { - if (statement.type === 'VariableDeclaration') { - for (const declarator of statement.declarations) { - registerDeclaration( - declarator, - declarator, - getPatternIdentifierNames(declarator.id) - ); - } - continue; - } - if ( - statement.type === 'FunctionDeclaration' || - statement.type === 'ClassDeclaration' - ) { - registerDeclaration(statement, statement, getDeclaredNames(statement)); - } - } - - return { declarationsByNode, declarationsByName }; -}; - -const collectLiveTopLevelDeclarations = ( - program: AnyNode, - graph: TopLevelDeclarationGraph -): Set => { - const pendingNames: string[] = []; - - for (const statement of program.body ?? []) { - if (statement.type === 'VariableDeclaration') { - continue; - } - if (graph.declarationsByNode.has(statement)) { - continue; - } - for (const name of collectReferencedNames(statement)) { - pendingNames.push(name); - } - } - - // This is intentionally name-based and conservative: shadowing may retain a - // declaration, but it must never make a live declaration removable. - const visitedNames = new Set(); - const liveDeclarations = new Set(); - while (pendingNames.length > 0) { - const name = pendingNames.pop(); - if (!name || visitedNames.has(name)) { - continue; - } - visitedNames.add(name); - for (const declaration of graph.declarationsByName.get(name) ?? []) { - if (!liveDeclarations.has(declaration)) { - liveDeclarations.add(declaration); - for (const referencedName of declaration.referencedNames) { - pendingNames.push(referencedName); - } - } - } - } - - return liveDeclarations; -}; - -const declarationReferencesName = ( - declaration: TopLevelDeclaration, - names: ReadonlySet, - graph: TopLevelDeclarationGraph, - cache: Map, - visitedNames = new Set() -): boolean => { - const cached = cache.get(declaration); - if (cached !== undefined) { - return cached; - } - - for (const referencedName of declaration.referencedNames) { - if (names.has(referencedName)) { - cache.set(declaration, true); - return true; - } - if (visitedNames.has(referencedName)) { - continue; - } - visitedNames.add(referencedName); - for (const referencedDeclaration of graph.declarationsByName.get( - referencedName - ) ?? []) { - if ( - declarationReferencesName( - referencedDeclaration, - names, - graph, - cache, - visitedNames - ) - ) { - cache.set(declaration, true); - return true; - } - } - } - cache.set(declaration, false); - return false; -}; - -const removeNewlyDeadTopLevelDeclarations = ( - program: AnyNode, - graph: TopLevelDeclarationGraph, - previouslyLive: ReadonlySet, - removedExportReferencedNames: ReadonlySet -): void => { - const currentlyLive = collectLiveTopLevelDeclarations(program, graph); - const removedReferenceCache = new Map(); - const isRemovableDeadDeclaration = (node: AnyNode) => { - const declaration = graph.declarationsByNode.get(node); - if (!declaration || currentlyLive.has(declaration)) { - return false; - } - return ( - previouslyLive.has(declaration) || - declarationReferencesName( - declaration, - removedExportReferencedNames, - graph, - removedReferenceCache - ) - ); - }; - - program.body = program.body.filter((statement: AnyNode) => { - if (statement.type === 'VariableDeclaration') { - statement.declarations = statement.declarations.filter( - (declarator: AnyNode) => !isRemovableDeadDeclaration(declarator) - ); - return statement.declarations.length > 0; - } - return !isRemovableDeadDeclaration(statement); - }); -}; - -const hasRemovableExport = ( - program: AnyNode, - exportsToRemove: ReadonlySet -): boolean => { - for (const statement of program.body ?? []) { - if (statement.type === 'ExportAllDeclaration') { - const exportedName = statement.exported - ? getExportedName({ exported: statement.exported }) - : null; - if (!exportedName || exportsToRemove.has(exportedName)) { - return true; - } - continue; - } - - if (statement.type === 'ExportDefaultDeclaration') { - if (exportsToRemove.has('default')) { - return true; - } - continue; - } - - if (statement.type !== 'ExportNamedDeclaration') { - continue; - } - - for (const specifier of statement.specifiers ?? []) { - if (specifier.type !== 'ExportSpecifier') { - continue; - } - const exportedName = getExportedName(specifier); - if (exportedName && exportsToRemove.has(exportedName)) { - return true; - } - } - - const declaration = statement.declaration; - if (declaration?.type === 'VariableDeclaration') { - for (const declarator of declaration.declarations ?? []) { - for (const name of getPatternIdentifierNames(declarator.id)) { - if (exportsToRemove.has(name)) { - return true; - } - } - } - continue; - } - - if ( - (declaration?.type === 'FunctionDeclaration' || - declaration?.type === 'ClassDeclaration') && - declaration.id?.name && - exportsToRemove.has(declaration.id.name) - ) { - return true; - } - } - return false; -}; - -export const removeExports = ( - ast: ParseResult | AnyNode, - exportsToRemove: readonly string[], - exportsToRemoveSet: ReadonlySet = new Set(exportsToRemove) -): boolean => { - const program = getProgram(ast); - if (!hasRemovableExport(program, exportsToRemoveSet)) { - return false; - } - - const declarationGraph = createTopLevelDeclarationGraph(program); - const previouslyLive = collectLiveTopLevelDeclarations( - program, - declarationGraph - ); - let exportsChanged = false; - const removedExportLocalNames = new Set(); - const removedExportReferencedNames = new Set(); - const trackRemovedExportReferences = (node: AnyNode | null | undefined) => { - if (!node) { - return; - } - const declaration = declarationGraph.declarationsByNode.get(node); - for (const name of declaration?.referencedNames ?? - collectReferencedNames(node)) { - removedExportReferencedNames.add(name); - } - }; - - for (const statement of [...program.body]) { - if (statement.type === 'ExportAllDeclaration') { - const exportedName = statement.exported - ? getExportedName({ exported: statement.exported }) - : null; - if (!exportedName || exportsToRemoveSet.has(exportedName)) { - exportsChanged = true; - removeFromArray(program.body, statement); - } - continue; - } - - if (statement.type === 'ExportNamedDeclaration') { - if (statement.specifiers?.length) { - statement.specifiers = statement.specifiers.filter( - (specifier: AnyNode) => { - if (specifier.type !== 'ExportSpecifier') { - return true; - } - const exportedName = getExportedName(specifier); - if (exportedName && exportsToRemoveSet.has(exportedName)) { - exportsChanged = true; - if (specifier.local?.name) { - removedExportLocalNames.add(specifier.local.name); - removedExportReferencedNames.add(specifier.local.name); - } - return false; - } - return true; - } - ); - if (statement.specifiers.length === 0 && !statement.declaration) { - removeFromArray(program.body, statement); - } - } - - const declaration = statement.declaration; - if (declaration?.type === 'VariableDeclaration') { - declaration.declarations = declaration.declarations.filter( - (declarator: AnyNode) => { - if (declarator.id.type === 'Identifier') { - if (exportsToRemoveSet.has(declarator.id.name)) { - exportsChanged = true; - removedExportLocalNames.add(declarator.id.name); - removedExportReferencedNames.add(declarator.id.name); - trackRemovedExportReferences(declarator); - return false; - } - return true; - } - - validateDestructuredExports(declarator.id, exportsToRemove); - return true; - } - ); - if (declaration.declarations.length === 0) { - removeFromArray(program.body, statement); - } - } - - if ( - (declaration?.type === 'FunctionDeclaration' || - declaration?.type === 'ClassDeclaration') && - declaration.id?.name && - exportsToRemoveSet.has(declaration.id.name) - ) { - exportsChanged = true; - removedExportLocalNames.add(declaration.id.name); - removedExportReferencedNames.add(declaration.id.name); - trackRemovedExportReferences(statement); - removeFromArray(program.body, statement); - } - } - - if ( - statement.type === 'ExportDefaultDeclaration' && - exportsToRemoveSet.has('default') - ) { - exportsChanged = true; - const declaration = statement.declaration; - if (declaration?.type === 'Identifier') { - removedExportLocalNames.add(declaration.name); - removedExportReferencedNames.add(declaration.name); - } else if (declaration?.id?.name) { - removedExportLocalNames.add(declaration.id.name); - removedExportReferencedNames.add(declaration.id.name); - } - trackRemovedExportReferences(statement); - removeFromArray(program.body, statement); - } - } - - for (const statement of [...program.body]) { - const expression = - statement.type === 'ExpressionStatement' ? statement.expression : null; - const left = - expression?.type === 'AssignmentExpression' ? expression.left : null; - if ( - left?.type === 'MemberExpression' && - left.object?.type === 'Identifier' && - removedExportLocalNames.has(left.object.name) - ) { - removeFromArray(program.body, statement); - } - } - - if (exportsChanged) { - removeNewlyDeadTopLevelDeclarations( - program, - declarationGraph, - previouslyLive, - removedExportReferencedNames - ); - } - - return exportsChanged; -}; - -export const removeUnusedImports = (ast: ParseResult | AnyNode): void => { - const program = getProgram(ast); - const referenced = collectReferencedNames(program); - for (const statement of [...program.body]) { - if (statement.type !== 'ImportDeclaration') { - continue; - } - if ((statement.specifiers ?? []).length === 0) { - continue; - } - statement.specifiers = (statement.specifiers ?? []).filter( - (specifier: AnyNode) => { - if (specifier.importKind === 'type') { - return false; - } - return !specifier.local?.name || referenced.has(specifier.local.name); - } - ); - if (statement.specifiers.length === 0) { - removeFromArray(program.body, statement); - } - } -}; - -const identifier = (name: string): AnyNode => ({ - type: 'Identifier', - start: 0, - end: 0, - name, - decorators: [], - optional: false, - typeAnnotation: null, -}); - -const literal = (value: string): AnyNode => ({ - type: 'Literal', - start: 0, - end: 0, - value, - raw: JSON.stringify(value), -}); - -const callExpression = (callee: AnyNode, args: AnyNode[]): AnyNode => ({ - type: 'CallExpression', - start: 0, - end: 0, - callee, - arguments: args, - optional: false, -}); - -const importDeclaration = ( - specifiers: Array<{ local: string; imported: string }>, - source: string -): AnyNode => ({ - type: 'ImportDeclaration', - start: 0, - end: 0, - specifiers: specifiers.map(specifier => ({ - type: 'ImportSpecifier', - start: 0, - end: 0, - imported: identifier(specifier.imported), - local: identifier(specifier.local), - importKind: 'value', - })), - source: literal(source), - attributes: [], - phase: null, - importKind: 'value', -}); - -const variableDeclaration = (name: string, init: AnyNode): AnyNode => ({ - type: 'VariableDeclaration', - start: 0, - end: 0, - kind: 'const', - declare: false, - declarations: [ - { - type: 'VariableDeclarator', - start: 0, - end: 0, - id: identifier(name), - init, - definite: false, - }, - ], -}); - -const patternIncludesName = ( - pattern: AnyNode | null | undefined, - name: string -): boolean => { - if (!pattern) { - return false; - } - if (pattern.type === 'Identifier') { - return pattern.name === name; - } - if (pattern.type === 'RestElement') { - return patternIncludesName(pattern.argument, name); - } - if (pattern.type === 'AssignmentPattern') { - return patternIncludesName(pattern.left, name); - } - if (pattern.type === 'ArrayPattern') { - return (pattern.elements ?? []).some((element: AnyNode | null) => - patternIncludesName(element, name) - ); - } - if (pattern.type === 'ObjectPattern') { - return (pattern.properties ?? []).some((property: AnyNode) => - property.type === 'RestElement' - ? patternIncludesName(property.argument, name) - : patternIncludesName(property.value, name) - ); - } - return false; -}; - -const declarationIncludesName = ( - declaration: AnyNode, - name: string -): boolean => { - if (declaration.type === 'VariableDeclaration') { - return (declaration.declarations ?? []).some((declarator: AnyNode) => - patternIncludesName(declarator.id, name) - ); - } - if ( - (declaration.type === 'FunctionDeclaration' || - declaration.type === 'ClassDeclaration') && - declaration.id?.name - ) { - return declaration.id.name === name; - } - if (declaration.type === 'ImportDeclaration') { - return (declaration.specifiers ?? []).some( - (specifier: AnyNode) => specifier.local?.name === name - ); - } - return false; -}; - -const hasTopLevelBindingName = (program: AnyNode, name: string): boolean => { - for (const statement of program.body ?? []) { - if (statement.type === 'ImportDeclaration') { - if (declarationIncludesName(statement, name)) { - return true; - } - continue; - } - - if (statement.type === 'ExportDefaultDeclaration') { - if (statement.declaration?.id?.name === name) { - return true; - } - continue; - } - - const declaration = - statement.type === 'ExportNamedDeclaration' - ? statement.declaration - : statement; - if (declaration && declarationIncludesName(declaration, name)) { - return true; - } - } - return false; -}; - -export const transformRoute = (ast: ParseResult | AnyNode): void => { - const program = getProgram(ast); - const usedNames = new Set(); - const hocs: Array<[string, string]> = []; - const componentWrapperDeclarations: AnyNode[] = []; - - function getUid(name: string) { - let uid = `_${name}`; - let index = 2; - while (usedNames.has(uid) || hasTopLevelBindingName(program, uid)) { - uid = `_${name}${index++}`; - } - usedNames.add(uid); - return uid; - } - - function getHocUid(hocName: string) { - const uid = getUid(hocName); - hocs.push([hocName, uid]); - return identifier(uid); - } - - function wrapNamedComponentDeclaration(name: string, declaration: AnyNode) { - const uid = getHocUid(`with${name}Props`); - const expression = - declaration.type === 'FunctionDeclaration' - ? toFunctionExpression(declaration) - : declaration.type === 'ClassDeclaration' - ? toClassExpression(declaration) - : declaration; - return variableDeclaration(name, callExpression(uid, [expression])); - } - - for (const statement of program.body ?? []) { - if (statement.type === 'ExportDefaultDeclaration') { - const declaration = statement.declaration; - const expr = - declaration?.type === 'FunctionDeclaration' - ? toFunctionExpression(declaration) - : declaration?.type === 'ClassDeclaration' - ? toClassExpression(declaration) - : declaration; - if (expr) { - const uid = getHocUid('withComponentProps'); - statement.declaration = callExpression(uid, [expr]); - } - continue; - } - - if (statement.type !== 'ExportNamedDeclaration') { - continue; - } - const declaration = statement.declaration; - if (declaration?.type === 'VariableDeclaration') { - for (const declarator of declaration.declarations ?? []) { - if ( - declarator.id?.type !== 'Identifier' || - !declarator.init || - !isNamedComponentExport(declarator.id.name) - ) { - continue; - } - const uid = getHocUid(`with${declarator.id.name}Props`); - declarator.init = callExpression(uid, [declarator.init]); - } - continue; - } - - if ( - (declaration?.type === 'FunctionDeclaration' || - declaration?.type === 'ClassDeclaration') && - declaration.id?.name && - isNamedComponentExport(declaration.id.name) - ) { - const name = declaration.id.name; - statement.declaration = wrapNamedComponentDeclaration(name, declaration); - continue; - } - - for (const specifier of statement.specifiers ?? []) { - if ( - specifier.type !== 'ExportSpecifier' || - specifier.exportKind === 'type' - ) { - continue; - } - const exportedName = getExportedName(specifier); - if (!exportedName || !isNamedComponentExport(exportedName)) { - continue; - } - const localName = specifier.local?.name; - if (!localName) { - continue; - } - const wrappedLocalName = getUid(exportedName); - const uid = getHocUid(`with${exportedName}Props`); - componentWrapperDeclarations.push( - variableDeclaration( - wrappedLocalName, - callExpression(uid, [identifier(localName)]) - ) - ); - specifier.local = identifier(wrappedLocalName); - } - } - - program.body.push(...componentWrapperDeclarations); - - if (hocs.length > 0) { - program.body.unshift( - importDeclaration( - hocs.map(([name, local]) => ({ imported: name, local })), - 'virtual/react-router/with-props' - ) - ); - } -}; - -function isNamedComponentExport( - name: string -): name is (typeof NAMED_COMPONENT_EXPORTS)[number] { - return NAMED_COMPONENT_EXPORTS_SET.has(name); -} +export { + invalidDestructureError, + removeExports, + removeUnusedImports, + validateDestructuredExports, +} from './route-export-pruning.js'; +export { transformRoute } from './route-component-transform.js'; diff --git a/src/prerender.ts b/src/prerender.ts index 9fbde67..8b41ab0 100644 --- a/src/prerender.ts +++ b/src/prerender.ts @@ -1,4 +1,3 @@ -import { getDefaultConcurrency } from './concurrency.js'; import type { Config } from './react-router-config.js'; import type { RouteConfigEntry } from '@react-router/dev/routes'; @@ -137,7 +136,7 @@ export const resolvePrerenderPaths = async ( export const getPrerenderConcurrency = ( prerender: PrerenderConfig, - cpuCount?: number + _cpuCount?: number ): number => { if ( typeof prerender === 'object' && @@ -149,7 +148,9 @@ export const getPrerenderConcurrency = ( return value; } } - return getDefaultConcurrency(cpuCount); + // Match React Router's default. Parallel prerendering can fan out loaders and + // external requests, so it must remain explicitly opt-in. + return 1; }; const isValidPrerenderPathsConfig = ( diff --git a/src/route-chunks.ts b/src/route-chunks.ts index 59f1b4c..e8c47b7 100644 --- a/src/route-chunks.ts +++ b/src/route-chunks.ts @@ -3,7 +3,7 @@ import { type Module, type Symbol as YukuSymbol, } from 'yuku-analyzer'; -import { strip } from 'yuku-codegen'; +import { print } from 'yuku-codegen'; import { walk } from 'yuku-parser'; import { normalize, relative, resolve } from 'pathe'; @@ -126,6 +126,7 @@ const analyzeCode = ( const module = analyzer.addFile(cacheKey, code, { lang: 'tsx', sourceType: 'module', + attachComments: true, }); const errors = module.diagnostics.filter( diagnostic => diagnostic.severity === 'error' @@ -405,7 +406,7 @@ const generateCode = (program: AnyNode): string | undefined => { if (program.body.length === 0) { return undefined; } - const result = strip(program as any, { comments: 'some' }); + const result = print(program as any, { comments: true }); if (result.errors.length > 0) { throw new Error(result.errors.map(error => error.message).join('\n')); } diff --git a/src/route-component-transform.ts b/src/route-component-transform.ts new file mode 100644 index 0000000..b1c3d83 --- /dev/null +++ b/src/route-component-transform.ts @@ -0,0 +1,420 @@ +import { + NAMED_COMPONENT_EXPORTS, + NAMED_COMPONENT_EXPORTS_SET, +} from './constants.js'; +import type { ParseResult } from 'yuku-parser'; + +type AnyNode = Record; + +const getProgram = (ast: ParseResult | AnyNode): AnyNode => + (ast as ParseResult).program ?? ast; + +const getExportedName = (specifier: AnyNode): string | null => { + const exported = specifier.exported; + if (!exported) { + return null; + } + if (exported.type === 'Identifier') { + return exported.name; + } + if (exported.type === 'Literal') { + return String(exported.value); + } + return null; +}; + +export function toFunctionExpression(decl: AnyNode): AnyNode { + return { + ...decl, + type: 'FunctionExpression', + declare: undefined, + }; +} + +export function toClassExpression(decl: AnyNode): AnyNode { + return { + ...decl, + type: 'ClassExpression', + declare: undefined, + }; +} + +const identifier = (name: string): AnyNode => ({ + type: 'Identifier', + start: 0, + end: 0, + name, + decorators: [], + optional: false, + typeAnnotation: null, +}); + +const literal = (value: string): AnyNode => ({ + type: 'Literal', + start: 0, + end: 0, + value, + raw: JSON.stringify(value), +}); + +const callExpression = (callee: AnyNode, args: AnyNode[]): AnyNode => ({ + type: 'CallExpression', + start: 0, + end: 0, + callee, + arguments: args, + optional: false, +}); + +const importDeclaration = ( + specifiers: Array<{ local: string; imported: string }>, + source: string +): AnyNode => ({ + type: 'ImportDeclaration', + start: 0, + end: 0, + specifiers: specifiers.map(specifier => ({ + type: 'ImportSpecifier', + start: 0, + end: 0, + imported: identifier(specifier.imported), + local: identifier(specifier.local), + importKind: 'value', + })), + source: literal(source), + attributes: [], + phase: null, + importKind: 'value', +}); + +const exportSpecifier = (local: string, exported: string): AnyNode => ({ + type: 'ExportSpecifier', + start: 0, + end: 0, + local: identifier(local), + exported: identifier(exported), + exportKind: 'value', +}); + +const exportNamedDeclaration = (specifiers: AnyNode[]): AnyNode => ({ + type: 'ExportNamedDeclaration', + start: 0, + end: 0, + declaration: null, + specifiers, + source: null, + attributes: [], + exportKind: 'value', +}); + +const getModuleExportName = ( + node: AnyNode | null | undefined +): string | null => { + if (!node) { + return null; + } + if (node.type === 'Identifier') { + return node.name; + } + if (node.type === 'Literal') { + return String(node.value); + } + return null; +}; + +const variableDeclaration = (name: string, init: AnyNode): AnyNode => ({ + type: 'VariableDeclaration', + start: 0, + end: 0, + kind: 'const', + declare: false, + declarations: [ + { + type: 'VariableDeclarator', + start: 0, + end: 0, + id: identifier(name), + init, + definite: false, + }, + ], +}); + +const patternIncludesName = ( + pattern: AnyNode | null | undefined, + name: string +): boolean => { + if (!pattern) { + return false; + } + if (pattern.type === 'Identifier') { + return pattern.name === name; + } + if (pattern.type === 'RestElement') { + return patternIncludesName(pattern.argument, name); + } + if (pattern.type === 'AssignmentPattern') { + return patternIncludesName(pattern.left, name); + } + if (pattern.type === 'ArrayPattern') { + return (pattern.elements ?? []).some((element: AnyNode | null) => + patternIncludesName(element, name) + ); + } + if (pattern.type === 'ObjectPattern') { + return (pattern.properties ?? []).some((property: AnyNode) => + property.type === 'RestElement' + ? patternIncludesName(property.argument, name) + : patternIncludesName(property.value, name) + ); + } + return false; +}; + +const declarationIncludesName = ( + declaration: AnyNode, + name: string +): boolean => { + if (declaration.type === 'VariableDeclaration') { + return (declaration.declarations ?? []).some((declarator: AnyNode) => + patternIncludesName(declarator.id, name) + ); + } + if ( + (declaration.type === 'FunctionDeclaration' || + declaration.type === 'ClassDeclaration') && + declaration.id?.name + ) { + return declaration.id.name === name; + } + if (declaration.type === 'ImportDeclaration') { + return (declaration.specifiers ?? []).some( + (specifier: AnyNode) => specifier.local?.name === name + ); + } + return false; +}; + +const hasTopLevelBindingName = (program: AnyNode, name: string): boolean => { + for (const statement of program.body ?? []) { + if (statement.type === 'ImportDeclaration') { + if (declarationIncludesName(statement, name)) { + return true; + } + continue; + } + + if (statement.type === 'ExportDefaultDeclaration') { + if (statement.declaration?.id?.name === name) { + return true; + } + continue; + } + + const declaration = + statement.type === 'ExportNamedDeclaration' + ? statement.declaration + : statement; + if (declaration && declarationIncludesName(declaration, name)) { + return true; + } + } + return false; +}; + +export const transformRoute = (ast: ParseResult | AnyNode): void => { + const program = getProgram(ast); + const usedNames = new Set(); + const hocs: Array<[string, string]> = []; + const componentWrapperDeclarations: AnyNode[] = []; + + function getUid(name: string) { + let uid = `_${name}`; + let index = 2; + while (usedNames.has(uid) || hasTopLevelBindingName(program, uid)) { + uid = `_${name}${index++}`; + } + usedNames.add(uid); + return uid; + } + + function getHocUid(hocName: string) { + const uid = getUid(hocName); + hocs.push([hocName, uid]); + return identifier(uid); + } + + function wrapNamedComponentDeclaration(name: string, declaration: AnyNode) { + const uid = getHocUid(`with${name}Props`); + const expression = + declaration.type === 'FunctionDeclaration' + ? toFunctionExpression(declaration) + : declaration.type === 'ClassDeclaration' + ? toClassExpression(declaration) + : declaration; + return variableDeclaration(name, callExpression(uid, [expression])); + } + + for (const statement of [...(program.body ?? [])]) { + if (statement.type === 'ExportDefaultDeclaration') { + const declaration = statement.declaration; + if (!declaration) { + continue; + } + const uid = getHocUid('withComponentProps'); + if ( + (declaration.type === 'FunctionDeclaration' || + declaration.type === 'ClassDeclaration') && + declaration.id?.name + ) { + const statementIndex = program.body.indexOf(statement); + program.body.splice(statementIndex, 0, declaration); + statement.declaration = callExpression(uid, [ + identifier(declaration.id.name), + ]); + continue; + } + const expression = + declaration.type === 'FunctionDeclaration' + ? toFunctionExpression(declaration) + : declaration.type === 'ClassDeclaration' + ? toClassExpression(declaration) + : declaration; + statement.declaration = callExpression(uid, [expression]); + continue; + } + + if (statement.type !== 'ExportNamedDeclaration') { + continue; + } + if (statement.exportKind === 'type') { + continue; + } + const declaration = statement.declaration; + if (declaration?.type === 'VariableDeclaration') { + for (const declarator of declaration.declarations ?? []) { + if ( + declarator.id?.type !== 'Identifier' || + !declarator.init || + !isNamedComponentExport(declarator.id.name) + ) { + continue; + } + const uid = getHocUid(`with${declarator.id.name}Props`); + declarator.init = callExpression(uid, [declarator.init]); + } + continue; + } + + if ( + (declaration?.type === 'FunctionDeclaration' || + declaration?.type === 'ClassDeclaration') && + declaration.id?.name && + isNamedComponentExport(declaration.id.name) + ) { + const name = declaration.id.name; + statement.declaration = wrapNamedComponentDeclaration(name, declaration); + continue; + } + + if (statement.source) { + const importSpecifiers: Array<{ local: string; imported: string }> = []; + const sourceWrapperDeclarations: AnyNode[] = []; + const wrappedExportSpecifiers: AnyNode[] = []; + statement.specifiers = (statement.specifiers ?? []).filter( + (specifier: AnyNode) => { + if ( + specifier.type !== 'ExportSpecifier' || + specifier.exportKind === 'type' + ) { + return true; + } + const exportedName = getExportedName(specifier); + const importedName = getModuleExportName(specifier.local); + if ( + !exportedName || + !importedName || + !isNamedComponentExport(exportedName) + ) { + return true; + } + const sourceLocalName = getUid(`${exportedName}Source`); + const wrappedLocalName = getUid(exportedName); + const uid = getHocUid(`with${exportedName}Props`); + importSpecifiers.push({ + imported: importedName, + local: sourceLocalName, + }); + sourceWrapperDeclarations.push( + variableDeclaration( + wrappedLocalName, + callExpression(uid, [identifier(sourceLocalName)]) + ) + ); + wrappedExportSpecifiers.push( + exportSpecifier(wrappedLocalName, exportedName) + ); + return false; + } + ); + if (importSpecifiers.length > 0) { + const statementIndex = program.body.indexOf(statement); + const replacementStatements = [ + importDeclaration(importSpecifiers, String(statement.source.value)), + ]; + if (statement.specifiers.length > 0) { + replacementStatements.push(statement); + } + replacementStatements.push(...sourceWrapperDeclarations); + replacementStatements.push( + exportNamedDeclaration(wrappedExportSpecifiers) + ); + program.body.splice(statementIndex, 1, ...replacementStatements); + } + continue; + } + + for (const specifier of statement.specifiers ?? []) { + if ( + specifier.type !== 'ExportSpecifier' || + specifier.exportKind === 'type' + ) { + continue; + } + const exportedName = getExportedName(specifier); + if (!exportedName || !isNamedComponentExport(exportedName)) { + continue; + } + const localName = specifier.local?.name; + if (!localName) { + continue; + } + const wrappedLocalName = getUid(exportedName); + const uid = getHocUid(`with${exportedName}Props`); + componentWrapperDeclarations.push( + variableDeclaration( + wrappedLocalName, + callExpression(uid, [identifier(localName)]) + ) + ); + specifier.local = identifier(wrappedLocalName); + } + } + + program.body.push(...componentWrapperDeclarations); + + if (hocs.length > 0) { + program.body.unshift( + importDeclaration( + hocs.map(([name, local]) => ({ imported: name, local })), + 'virtual/react-router/with-props' + ) + ); + } +}; + +function isNamedComponentExport( + name: string +): name is (typeof NAMED_COMPONENT_EXPORTS)[number] { + return NAMED_COMPONENT_EXPORTS_SET.has(name); +} diff --git a/src/route-export-pruning.ts b/src/route-export-pruning.ts new file mode 100644 index 0000000..72fd89a --- /dev/null +++ b/src/route-export-pruning.ts @@ -0,0 +1,704 @@ +import { walk, type ParseResult } from 'yuku-parser'; + +type AnyNode = Record; + +const getProgram = (ast: ParseResult | AnyNode): AnyNode => + (ast as ParseResult).program ?? ast; + +export function validateDestructuredExports( + id: AnyNode, + exportsToRemove: readonly string[] +): void { + if (id.type === 'Identifier') { + if (exportsToRemove.includes(id.name)) { + throw invalidDestructureError(id.name); + } + return; + } + + if (id.type === 'AssignmentPattern') { + validateDestructuredExports(id.left, exportsToRemove); + return; + } + + if (id.type === 'ArrayPattern') { + for (const element of id.elements ?? []) { + if (!element) { + continue; + } + + if (element.type === 'AssignmentPattern') { + validateDestructuredExports(element, exportsToRemove); + continue; + } + + if ( + element.type === 'Identifier' && + exportsToRemove.includes(element.name) + ) { + throw invalidDestructureError(element.name); + } + + if ( + element.type === 'RestElement' && + element.argument.type === 'Identifier' && + exportsToRemove.includes(element.argument.name) + ) { + throw invalidDestructureError(element.argument.name); + } + + if (element.type === 'ArrayPattern' || element.type === 'ObjectPattern') { + validateDestructuredExports(element, exportsToRemove); + } + } + } + + if (id.type === 'ObjectPattern') { + for (const property of id.properties ?? []) { + if (!property) { + continue; + } + + if (property.type === 'Property') { + if ( + property.value.type === 'Identifier' && + exportsToRemove.includes(property.value.name) + ) { + throw invalidDestructureError(property.value.name); + } + + if ( + property.value.type === 'AssignmentPattern' || + property.value.type === 'ArrayPattern' || + property.value.type === 'ObjectPattern' + ) { + validateDestructuredExports(property.value, exportsToRemove); + } + } + + if ( + property.type === 'RestElement' && + property.argument.type === 'Identifier' && + exportsToRemove.includes(property.argument.name) + ) { + throw invalidDestructureError(property.argument.name); + } + } + } +} + +export function invalidDestructureError(name: string): Error { + return new Error(`Cannot remove destructured export "${name}"`); +} + +const removeFromArray = (array: T[], value: T): void => { + const index = array.indexOf(value); + if (index >= 0) { + array.splice(index, 1); + } +}; + +const getPatternIdentifierNames = ( + pattern: AnyNode | null | undefined, + names = new Set() +): Set => { + if (!pattern) { + return names; + } + if (pattern.type === 'Identifier') { + names.add(pattern.name); + return names; + } + if (pattern.type === 'RestElement') { + return getPatternIdentifierNames(pattern.argument, names); + } + if (pattern.type === 'AssignmentPattern') { + return getPatternIdentifierNames(pattern.left, names); + } + if (pattern.type === 'ArrayPattern') { + for (const element of pattern.elements ?? []) { + getPatternIdentifierNames(element, names); + } + return names; + } + if (pattern.type === 'ObjectPattern') { + for (const property of pattern.properties ?? []) { + if (property.type === 'RestElement') { + getPatternIdentifierNames(property.argument, names); + } else { + getPatternIdentifierNames(property.value, names); + } + } + } + return names; +}; + +const getDeclaredNames = (node: AnyNode): Set => { + const names = new Set(); + if (node.type === 'VariableDeclaration') { + for (const declarator of node.declarations ?? []) { + getPatternIdentifierNames(declarator.id, names); + } + } else if ( + (node.type === 'FunctionDeclaration' || node.type === 'ClassDeclaration') && + node.id?.name + ) { + names.add(node.id.name); + } else if (node.type === 'ImportDeclaration') { + for (const specifier of node.specifiers ?? []) { + if (specifier.local?.name) { + names.add(specifier.local.name); + } + } + } + return names; +}; + +const isIdentifierDeclaration = (node: AnyNode, parent: AnyNode | null) => { + if (!parent || node.type !== 'Identifier') { + return false; + } + if ( + (parent.type === 'FunctionDeclaration' || + parent.type === 'FunctionExpression' || + parent.type === 'ClassDeclaration' || + parent.type === 'ClassExpression') && + parent.id === node + ) { + return true; + } + if (parent.type === 'VariableDeclarator') { + return getPatternIdentifierNames(parent.id).has(node.name); + } + if ( + (parent.type === 'ImportSpecifier' || + parent.type === 'ImportDefaultSpecifier' || + parent.type === 'ImportNamespaceSpecifier') && + parent.local === node + ) { + return true; + } + if ( + (parent.type === 'FunctionDeclaration' || + parent.type === 'FunctionExpression' || + parent.type === 'ArrowFunctionExpression') && + (parent.params ?? []).some((param: AnyNode) => + getPatternIdentifierNames(param).has(node.name) + ) + ) { + return true; + } + return false; +}; + +const isNonReferenceIdentifier = (node: AnyNode, parent: AnyNode | null) => { + if (!parent || node.type !== 'Identifier') { + return false; + } + if (isIdentifierDeclaration(node, parent)) { + return true; + } + if ( + parent.type === 'MemberExpression' && + parent.property === node && + !parent.computed + ) { + return true; + } + if ( + parent.type === 'Property' && + parent.key === node && + !parent.computed && + !parent.shorthand + ) { + return true; + } + if ( + parent.type === 'MethodDefinition' && + parent.key === node && + !parent.computed + ) { + return true; + } + if ( + parent.type === 'ExportSpecifier' || + parent.type === 'ExportDefaultSpecifier' || + parent.type === 'ExportNamespaceSpecifier' + ) { + return true; + } + if (parent.type === 'ImportSpecifier' && parent.imported === node) { + return true; + } + if ( + parent.type === 'LabeledStatement' || + parent.type === 'BreakStatement' || + parent.type === 'ContinueStatement' + ) { + return true; + } + return false; +}; + +const isUppercaseName = (name: string): boolean => /^[A-Z]/.test(name); + +const collectReferencedNames = (node: AnyNode): Set => { + const referenced = new Set(); + walk(node as any, { + Identifier(node: AnyNode, ctx: any) { + const parent = ctx.parent as AnyNode | null; + if (!isNonReferenceIdentifier(node, parent)) { + referenced.add(node.name); + } + }, + JSXIdentifier(node: AnyNode, ctx: any) { + const parent = ctx.parent as AnyNode | null; + if (!parent) { + return; + } + if (parent.type === 'JSXMemberExpression' && parent.object === node) { + referenced.add(node.name); + return; + } + if (!isUppercaseName(node.name)) { + return; + } + if ( + (parent.type === 'JSXOpeningElement' || + parent.type === 'JSXClosingElement') && + parent.name === node + ) { + referenced.add(node.name); + return; + } + }, + ExportSpecifier(node: AnyNode, ctx: any) { + const declaration = ctx.parent as AnyNode | null; + if ( + !declaration?.source && + declaration?.exportKind !== 'type' && + node.local?.name && + node.exportKind !== 'type' + ) { + referenced.add(node.local.name); + } + }, + }); + return referenced; +}; + +const getExportedName = (specifier: AnyNode): string | null => { + const exported = specifier.exported; + if (!exported) { + return null; + } + if (exported.type === 'Identifier') { + return exported.name; + } + if (exported.type === 'Literal') { + return String(exported.value); + } + return null; +}; + +type TopLevelDeclaration = { + referencedNames: Set; +}; + +type TopLevelDeclarationGraph = { + declarationsByNode: Map; + declarationsByName: Map>; +}; + +const createTopLevelDeclarationGraph = ( + program: AnyNode +): TopLevelDeclarationGraph => { + const declarationsByNode = new Map(); + const declarationsByName = new Map>(); + + const registerDeclaration = ( + node: AnyNode, + declarationNode: AnyNode, + declaredNames: Set + ) => { + const declaration: TopLevelDeclaration = { + referencedNames: collectReferencedNames(declarationNode), + }; + declarationsByNode.set(node, declaration); + for (const name of declaredNames) { + const namedDeclarations = declarationsByName.get(name) ?? new Set(); + namedDeclarations.add(declaration); + declarationsByName.set(name, namedDeclarations); + } + }; + + for (const statement of [...(program.body ?? [])]) { + if (statement.type === 'VariableDeclaration') { + for (const declarator of statement.declarations) { + registerDeclaration( + declarator, + declarator, + getPatternIdentifierNames(declarator.id) + ); + } + continue; + } + if ( + statement.type === 'FunctionDeclaration' || + statement.type === 'ClassDeclaration' + ) { + registerDeclaration(statement, statement, getDeclaredNames(statement)); + } + } + + return { declarationsByNode, declarationsByName }; +}; + +const collectLiveTopLevelDeclarations = ( + program: AnyNode, + graph: TopLevelDeclarationGraph +): Set => { + const pendingNames: string[] = []; + + for (const statement of program.body ?? []) { + if (statement.type === 'VariableDeclaration') { + continue; + } + if (graph.declarationsByNode.has(statement)) { + continue; + } + for (const name of collectReferencedNames(statement)) { + pendingNames.push(name); + } + } + + // This is intentionally name-based and conservative: shadowing may retain a + // declaration, but it must never make a live declaration removable. + const visitedNames = new Set(); + const liveDeclarations = new Set(); + while (pendingNames.length > 0) { + const name = pendingNames.pop(); + if (!name || visitedNames.has(name)) { + continue; + } + visitedNames.add(name); + for (const declaration of graph.declarationsByName.get(name) ?? []) { + if (!liveDeclarations.has(declaration)) { + liveDeclarations.add(declaration); + for (const referencedName of declaration.referencedNames) { + pendingNames.push(referencedName); + } + } + } + } + + return liveDeclarations; +}; + +const declarationReferencesName = ( + declaration: TopLevelDeclaration, + names: ReadonlySet, + graph: TopLevelDeclarationGraph, + cache: Map, + visitedNames = new Set() +): boolean => { + const cached = cache.get(declaration); + if (cached !== undefined) { + return cached; + } + + for (const referencedName of declaration.referencedNames) { + if (names.has(referencedName)) { + cache.set(declaration, true); + return true; + } + if (visitedNames.has(referencedName)) { + continue; + } + visitedNames.add(referencedName); + for (const referencedDeclaration of graph.declarationsByName.get( + referencedName + ) ?? []) { + if ( + declarationReferencesName( + referencedDeclaration, + names, + graph, + cache, + visitedNames + ) + ) { + cache.set(declaration, true); + return true; + } + } + } + cache.set(declaration, false); + return false; +}; + +const removeNewlyDeadTopLevelDeclarations = ( + program: AnyNode, + graph: TopLevelDeclarationGraph, + previouslyLive: ReadonlySet, + removedExportReferencedNames: ReadonlySet +): void => { + const currentlyLive = collectLiveTopLevelDeclarations(program, graph); + const removedReferenceCache = new Map(); + const isRemovableDeadDeclaration = (node: AnyNode) => { + const declaration = graph.declarationsByNode.get(node); + if (!declaration || currentlyLive.has(declaration)) { + return false; + } + return ( + previouslyLive.has(declaration) || + declarationReferencesName( + declaration, + removedExportReferencedNames, + graph, + removedReferenceCache + ) + ); + }; + + program.body = program.body.filter((statement: AnyNode) => { + if (statement.type === 'VariableDeclaration') { + statement.declarations = statement.declarations.filter( + (declarator: AnyNode) => !isRemovableDeadDeclaration(declarator) + ); + return statement.declarations.length > 0; + } + return !isRemovableDeadDeclaration(statement); + }); +}; + +const hasRemovableExport = ( + program: AnyNode, + exportsToRemove: ReadonlySet +): boolean => { + for (const statement of program.body ?? []) { + if (statement.type === 'ExportAllDeclaration') { + const exportedName = statement.exported + ? getExportedName({ exported: statement.exported }) + : null; + if (exportedName && exportsToRemove.has(exportedName)) { + return true; + } + continue; + } + + if (statement.type === 'ExportDefaultDeclaration') { + if (exportsToRemove.has('default')) { + return true; + } + continue; + } + + if (statement.type !== 'ExportNamedDeclaration') { + continue; + } + + for (const specifier of statement.specifiers ?? []) { + if (specifier.type !== 'ExportSpecifier') { + continue; + } + const exportedName = getExportedName(specifier); + if (exportedName && exportsToRemove.has(exportedName)) { + return true; + } + } + + const declaration = statement.declaration; + if (declaration?.type === 'VariableDeclaration') { + for (const declarator of declaration.declarations ?? []) { + for (const name of getPatternIdentifierNames(declarator.id)) { + if (exportsToRemove.has(name)) { + return true; + } + } + } + continue; + } + + if ( + (declaration?.type === 'FunctionDeclaration' || + declaration?.type === 'ClassDeclaration') && + declaration.id?.name && + exportsToRemove.has(declaration.id.name) + ) { + return true; + } + } + return false; +}; + +export const removeExports = ( + ast: ParseResult | AnyNode, + exportsToRemove: readonly string[], + exportsToRemoveSet: ReadonlySet = new Set(exportsToRemove) +): boolean => { + const program = getProgram(ast); + if (!hasRemovableExport(program, exportsToRemoveSet)) { + return false; + } + + const declarationGraph = createTopLevelDeclarationGraph(program); + const previouslyLive = collectLiveTopLevelDeclarations( + program, + declarationGraph + ); + let exportsChanged = false; + const removedExportLocalNames = new Set(); + const removedExportReferencedNames = new Set(); + const trackRemovedExportReferences = (node: AnyNode | null | undefined) => { + if (!node) { + return; + } + const declaration = declarationGraph.declarationsByNode.get(node); + for (const name of declaration?.referencedNames ?? + collectReferencedNames(node)) { + removedExportReferencedNames.add(name); + } + }; + + for (const statement of [...program.body]) { + if (statement.type === 'ExportAllDeclaration') { + const exportedName = statement.exported + ? getExportedName({ exported: statement.exported }) + : null; + if (exportedName && exportsToRemoveSet.has(exportedName)) { + exportsChanged = true; + removeFromArray(program.body, statement); + } + continue; + } + + if (statement.type === 'ExportNamedDeclaration') { + if (statement.specifiers?.length) { + statement.specifiers = statement.specifiers.filter( + (specifier: AnyNode) => { + if (specifier.type !== 'ExportSpecifier') { + return true; + } + const exportedName = getExportedName(specifier); + if (exportedName && exportsToRemoveSet.has(exportedName)) { + exportsChanged = true; + if (specifier.local?.name) { + removedExportLocalNames.add(specifier.local.name); + removedExportReferencedNames.add(specifier.local.name); + } + return false; + } + return true; + } + ); + if (statement.specifiers.length === 0 && !statement.declaration) { + removeFromArray(program.body, statement); + } + } + + const declaration = statement.declaration; + if (declaration?.type === 'VariableDeclaration') { + declaration.declarations = declaration.declarations.filter( + (declarator: AnyNode) => { + if (declarator.id.type === 'Identifier') { + if (exportsToRemoveSet.has(declarator.id.name)) { + exportsChanged = true; + removedExportLocalNames.add(declarator.id.name); + removedExportReferencedNames.add(declarator.id.name); + trackRemovedExportReferences(declarator); + return false; + } + return true; + } + + validateDestructuredExports(declarator.id, exportsToRemove); + return true; + } + ); + if (declaration.declarations.length === 0) { + removeFromArray(program.body, statement); + } + } + + if ( + (declaration?.type === 'FunctionDeclaration' || + declaration?.type === 'ClassDeclaration') && + declaration.id?.name && + exportsToRemoveSet.has(declaration.id.name) + ) { + exportsChanged = true; + removedExportLocalNames.add(declaration.id.name); + removedExportReferencedNames.add(declaration.id.name); + trackRemovedExportReferences(statement); + removeFromArray(program.body, statement); + } + } + + if ( + statement.type === 'ExportDefaultDeclaration' && + exportsToRemoveSet.has('default') + ) { + exportsChanged = true; + const declaration = statement.declaration; + if (declaration?.type === 'Identifier') { + removedExportLocalNames.add(declaration.name); + removedExportReferencedNames.add(declaration.name); + } else if (declaration?.id?.name) { + removedExportLocalNames.add(declaration.id.name); + removedExportReferencedNames.add(declaration.id.name); + } + trackRemovedExportReferences(statement); + removeFromArray(program.body, statement); + } + } + + for (const statement of [...program.body]) { + const expression = + statement.type === 'ExpressionStatement' ? statement.expression : null; + const left = + expression?.type === 'AssignmentExpression' ? expression.left : null; + if ( + left?.type === 'MemberExpression' && + left.object?.type === 'Identifier' && + removedExportLocalNames.has(left.object.name) + ) { + removeFromArray(program.body, statement); + } + } + + if (exportsChanged) { + removeNewlyDeadTopLevelDeclarations( + program, + declarationGraph, + previouslyLive, + removedExportReferencedNames + ); + } + + return exportsChanged; +}; + +export const removeUnusedImports = (ast: ParseResult | AnyNode): void => { + const program = getProgram(ast); + const referenced = collectReferencedNames(program); + for (const statement of [...program.body]) { + if (statement.type !== 'ImportDeclaration') { + continue; + } + if ((statement.specifiers ?? []).length === 0) { + continue; + } + statement.specifiers = (statement.specifiers ?? []).filter( + (specifier: AnyNode) => { + if (specifier.importKind === 'type') { + return false; + } + return !specifier.local?.name || referenced.has(specifier.local.name); + } + ); + if (statement.specifiers.length === 0) { + removeFromArray(program.body, statement); + } + } +}; diff --git a/src/route-transform-tasks.ts b/src/route-transform-tasks.ts index 7375304..1bb134d 100644 --- a/src/route-transform-tasks.ts +++ b/src/route-transform-tasks.ts @@ -2,7 +2,7 @@ import { statSync, type Stats } from 'node:fs'; import { createRequire } from 'node:module'; import { pathToFileURL } from 'node:url'; import { basename as pathBasename, dirname, relative, resolve } from 'pathe'; -import { generate, parse } from './babel.js'; +import { generate, parse } from './yuku.js'; import { JS_EXTENSIONS, PLUGIN_NAME, @@ -67,6 +67,7 @@ export type RouteModuleTransformTask = BaseRouteTransformTask & { kind: 'routeModule'; resource: string; environmentName: string; + sourceMaps: boolean; ssr: boolean; isBuild: boolean; isSpaMode: boolean; @@ -312,7 +313,9 @@ const transformRouteModule = async ( } return generate(ast, { - sourceMaps: !task.isBuild, + // Rsbuild merges this map with its downstream SWC transform. Only pay the + // code-generation cost when this environment actually emits JS maps. + sourceMaps: task.sourceMaps, filename: task.resource, sourceFileName: task.resourcePath, }); diff --git a/src/route-watch.ts b/src/route-watch.ts index d57cc81..e726fec 100644 --- a/src/route-watch.ts +++ b/src/route-watch.ts @@ -1,10 +1,10 @@ -import { readFileSync, watch, type FSWatcher } from 'node:fs'; +import { watch, type FSWatcher } from 'node:fs'; import { access, mkdir, readdir, writeFile } from 'node:fs/promises'; -import type { ProcessAssetsHandler, RsbuildConfig } from '@rsbuild/core'; +import type { RsbuildConfig } from '@rsbuild/core'; import { dirname, resolve } from 'pathe'; import type { Route } from './types.js'; -export const ROUTE_RESTART_MARKER_ASSET = '.react-router/route-watch'; +const ROUTE_RESTART_MARKER_ASSET = '.react-router/route-watch'; const INITIAL_RESTART_MARKER_CONTENT = 'react-router-route-watch'; type RouteManifestSnapshotEntry = Pick< @@ -24,12 +24,21 @@ type RouteDirectoryState = { routeTopology: Set; }; -type ProcessAssetsContext = Parameters[0]; -type RouteRestartMarkerAssetOptions = Pick< - ProcessAssetsContext, - 'compilation' | 'sources' -> & { - restartMarkerPath: string; +type DirectoryWatcher = Pick; +type WatchDirectoryEntry = ( + directory: string, + onChange: () => void, + onError: (error: unknown) => void +) => DirectoryWatcher; + +const defaultWatchDirectoryEntry: WatchDirectoryEntry = ( + directory, + onChange, + onError +) => { + const watcher = watch(directory, onChange); + watcher.on('error', onError); + return watcher; }; export const mergeWatchFiles = ( @@ -48,30 +57,6 @@ export const mergeWatchFiles = ( export const getRouteRestartMarkerPath = (outputClientPath: string): string => resolve(outputClientPath, ROUTE_RESTART_MARKER_ASSET); -const readRestartMarkerContent = (restartMarkerPath: string): string => { - try { - const content = readFileSync(restartMarkerPath, 'utf8'); - return content || INITIAL_RESTART_MARKER_CONTENT; - } catch { - return INITIAL_RESTART_MARKER_CONTENT; - } -}; - -export const emitRouteRestartMarkerAsset = ({ - restartMarkerPath, - sources, - compilation, -}: RouteRestartMarkerAssetOptions): void => { - const source = new sources.RawSource( - readRestartMarkerContent(restartMarkerPath) - ); - if (compilation.getAsset(ROUTE_RESTART_MARKER_ASSET)) { - compilation.updateAsset(ROUTE_RESTART_MARKER_ASSET, source); - return; - } - compilation.emitAsset(ROUTE_RESTART_MARKER_ASSET, source); -}; - export const createRouteManifestSnapshot = ( routes: Record ): Set => @@ -94,8 +79,8 @@ export const createRouteManifestSnapshot = ( export const ensureDevRestartMarker = async ( restartMarkerPath: string ): Promise => { - // Build emits this marker through processAssets. Dev owns the watched file - // directly so ordinary rebuilds do not rewrite it and trigger reload loops. + // Dev owns this watched file directly so ordinary rebuilds do not rewrite it + // and trigger reload loops. await mkdir(dirname(restartMarkerPath), { recursive: true }); try { await access(restartMarkerPath); @@ -154,22 +139,32 @@ const readRouteDirectoryState = async ({ export const createRouteTopologyWatcher = async ({ watchDirectory, getRouteTopology, + initialRouteTopology, restartMarkerPath, onError, + onRouteTopologyChange, + watchDirectoryEntry: watchDirectoryOverride = defaultWatchDirectoryEntry, }: { watchDirectory: string; getRouteTopology: () => Promise>; + initialRouteTopology?: Set; restartMarkerPath: string; onError: (error: unknown) => void; -}): Promise<() => void> => { - let state = await readRouteDirectoryState({ + onRouteTopologyChange?: () => void | Promise; + watchDirectoryEntry?: WatchDirectoryEntry; +}): Promise<() => Promise> => { + const discoveredState = await readRouteDirectoryState({ watchDirectory, getRouteTopology, }); + let state = { + ...discoveredState, + routeTopology: initialRouteTopology ?? discoveredState.routeTopology, + }; let closed = false; let rescanTimer: ReturnType | undefined; let rescanQueue = Promise.resolve(); - const directoryWatchers = new Map(); + const directoryWatchers = new Map(); const touchRestartMarker = async (): Promise => { await mkdir(dirname(restartMarkerPath), { recursive: true }); @@ -193,10 +188,14 @@ export const createRouteTopologyWatcher = async ({ continue; } try { - const watcher = watch(directory, () => { - scheduleRescan(); + let watcher: DirectoryWatcher; + watcher = watchDirectoryOverride(directory, scheduleRescan, error => { + if (directoryWatchers.get(directory) === watcher) { + watcher.close(); + directoryWatchers.delete(directory); + } + onError(error); }); - watcher.on('error', onError); directoryWatchers.set(directory, watcher); } catch (error) { onError(error); @@ -218,10 +217,26 @@ export const createRouteTopologyWatcher = async ({ watchDirectory, getRouteTopology, }); + if (closed) { + return; + } syncDirectoryWatchers(nextState.directories); if (!areSetsEqual(state.routeTopology, nextState.routeTopology)) { + if (onRouteTopologyChange) { + // This is a notification boundary, not part of the rescan + // transaction. A custom-server callback may close this watcher while + // replacing its compiler, so awaiting it here would deadlock close(). + const notification = onRouteTopologyChange(); + state = nextState; + void Promise.resolve(notification).catch(onError); + return; + } else { + await touchRestartMarker(); + } + if (closed) { + return; + } state = nextState; - await touchRestartMarker(); return; } state = nextState; @@ -246,8 +261,15 @@ export const createRouteTopologyWatcher = async ({ }; syncDirectoryWatchers(state.directories); + if (initialRouteTopology) { + await runRescan(); + } - return () => { + return async () => { + if (closed) { + await rescanQueue; + return; + } closed = true; if (rescanTimer) { clearTimeout(rescanTimer); @@ -256,5 +278,10 @@ export const createRouteTopologyWatcher = async ({ watcher.close(); } directoryWatchers.clear(); + await rescanQueue; + for (const watcher of directoryWatchers.values()) { + watcher.close(); + } + directoryWatchers.clear(); }; }; diff --git a/src/types.ts b/src/types.ts index ba40843..e046abf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -45,13 +45,22 @@ export type PluginOptions = { /** * Run route transforms in a worker-thread pool. * Pass `false` to disable or `{ maxWorkers }` to override the default worker count. - * @default true, using `available CPUs - 2` workers. + * @default Automatically enabled for 256+ resolved routes. The automatic + * pool is capped at 8 workers. */ parallelTransforms?: | boolean | { maxWorkers?: number; }; + + /** + * Called when the route graph changes during development. + * Programmatic/custom servers can use this to recreate their Rsbuild server; + * the CLI uses its built-in reload-server watcher when this is omitted. This + * notification is not awaited, so it may safely close the current server. + */ + onRouteTopologyChange?: () => void | Promise; }; export type RouteManifestItem = Omit & { diff --git a/src/warnings/warn-on-client-source-maps.ts b/src/warnings/warn-on-client-source-maps.ts index e595bb9..9696bf6 100644 --- a/src/warnings/warn-on-client-source-maps.ts +++ b/src/warnings/warn-on-client-source-maps.ts @@ -8,7 +8,7 @@ function isProdBuild(mode?: string): boolean { return mode === 'production' || process.env.NODE_ENV === 'production'; } -function isSourceMapEnabled(value: unknown): boolean { +export function isSourceMapEnabled(value: unknown): boolean { // Rsbuild normalizes `output.sourceMap` into either: // - boolean // - { js?: devtool; css: boolean } diff --git a/src/babel.ts b/src/yuku.ts similarity index 82% rename from src/babel.ts rename to src/yuku.ts index d14a254..18a9ccc 100644 --- a/src/babel.ts +++ b/src/yuku.ts @@ -5,15 +5,17 @@ import { type ParseResult, } from 'yuku-parser'; import type { Rspack } from '@rsbuild/core'; -import { strip } from 'yuku-codegen'; +import { print } from 'yuku-codegen'; export const parse = ( code: string, options: ParseOptions = {} ): ParseResult => { const result = yukuParse(code, { + ...options, sourceType: options.sourceType ?? 'module', lang: options.lang ?? 'tsx', + attachComments: options.attachComments ?? true, }); const errors = result.diagnostics.filter( diagnostic => diagnostic.severity === 'error' @@ -35,8 +37,8 @@ export const generate = ( } = {} ): { code: string; map: Rspack.RawSourceMap | null } => { const result = 'program' in ast ? ast : { program: ast, lineStarts: [] }; - const generated = strip(result.program as Parameters[0], { - comments: 'some', + const generated = print(result.program as Parameters[0], { + comments: true, sourceMaps: options.sourceMaps ? { lineStarts: result.lineStarts, @@ -45,6 +47,9 @@ export const generate = ( } : undefined, }); + if (generated.errors.length > 0) { + throw new Error(generated.errors.map(error => error.message).join('\n')); + } const map = generated.map ? { ...generated.map, @@ -59,5 +64,4 @@ export const generate = ( return { code: generated.code, map }; }; -export const t = {}; export type { ParseResult }; diff --git a/tests/export-utils.test.ts b/tests/export-utils.test.ts index 921d1d6..4dc18ce 100644 --- a/tests/export-utils.test.ts +++ b/tests/export-utils.test.ts @@ -1,64 +1,10 @@ import { describe, expect, it } from '@rstest/core'; -import { parse } from '../src/babel'; -import { - getBundlerRouteAnalysis, - getExportNamesAndExportAll, - transformToEsm, -} from '../src/export-utils'; +import { getExportNamesAndExportAll } from '../src/export-utils'; -const routeChunkConfig = { - splitRouteModules: true as const, - appDirectory: '/app', - rootRouteFile: 'root.tsx', -}; - -describe('getBundlerRouteAnalysis', () => { - it('reuses source code, export names, and chunk info for the same source', async () => { - const source = ` - export const clientAction = async () => {}; - export default function Route() { return null; } - `; - const resourcePath = '/app/routes/demo.tsx'; - - const first = await getBundlerRouteAnalysis(source, resourcePath); - const second = await getBundlerRouteAnalysis(source, resourcePath); - - expect(second).toBe(first); - expect(second.code).toBe(first.code); - expect(second.exportNames).toBe(first.exportNames); - expect(second.getRouteChunkInfo(undefined, routeChunkConfig)).toBe( - first.getRouteChunkInfo(undefined, routeChunkConfig) - ); - - expect(first.code).toBe(source); - expect(first.exportNames).toEqual(['clientAction', 'default']); +describe('getExportNamesAndExportAll', () => { + it('collects runtime exports and export-all modules', async () => { await expect( - first.getRouteChunkInfo(undefined, routeChunkConfig) - ).resolves.toMatchObject({ - hasRouteChunks: true, - chunkedExports: ['clientAction'], - }); - }); - - it('replaces the cached analysis when the source changes for the same resource', async () => { - const resourcePath = '/app/routes/demo.tsx'; - - const initial = await getBundlerRouteAnalysis( - `export const clientAction = async () => {};`, - resourcePath - ); - const updated = await getBundlerRouteAnalysis( - `export const clientLoader = async () => {};`, - resourcePath - ); - - expect(updated).not.toBe(initial); - expect(updated.exportNames).toEqual(['clientLoader']); - }); - - it('collects runtime exports and export-all modules from the initial parse', async () => { - const analysis = await getBundlerRouteAnalysis( - ` + getExportNamesAndExportAll(` export type LoaderData = { value: string }; export interface RouteHandle { title: string } export type * from './types'; @@ -67,88 +13,41 @@ describe('getBundlerRouteAnalysis', () => { export * as helpers from './helpers'; export const loader = () => null; export default function Route() { return null; } - `, - '/app/routes/runtime-exports.tsx' - ); - - const exportInfo = { - exportNames: analysis.exportNames, - exportAllModules: analysis.exportAllModules, - }; - expect(exportInfo).toEqual({ + `) + ).resolves.toEqual({ exportNames: ['helpers', 'loader', 'default'], exportAllModules: ['./shared'], }); - await expect(getExportNamesAndExportAll(analysis.code)).resolves.toEqual( - exportInfo - ); }); - it('collects exported TypeScript enum names as runtime exports', async () => { + it('collects exported TypeScript enums as runtime exports', async () => { await expect( - getExportNamesAndExportAll( - `export enum Status { Active = 'active' }` - ) + getExportNamesAndExportAll(`export enum Status { Active = 'active' }`) ).resolves.toEqual({ exportNames: ['Status'], exportAllModules: [], }); }); - it('does not report an erased default interface as a runtime export', async () => { - const analysis = await getBundlerRouteAnalysis( - `export default interface RouteType { value: string }`, - '/app/routes/type-only-default.tsx' - ); - const exportInfo = { - exportNames: analysis.exportNames, - exportAllModules: analysis.exportAllModules, - }; - - expect(exportInfo).toEqual({ exportNames: [], exportAllModules: [] }); - await expect(getExportNamesAndExportAll(analysis.code)).resolves.toEqual( - exportInfo - ); + it('ignores erased default interfaces', async () => { + await expect( + getExportNamesAndExportAll( + `export default interface RouteType { value: string }` + ) + ).resolves.toEqual({ exportNames: [], exportAllModules: [] }); }); - it('does not report erased ambient declarations as runtime exports', async () => { - const analysis = await getBundlerRouteAnalysis( - ` + it('ignores erased ambient declarations', async () => { + await expect( + getExportNamesAndExportAll(` export declare function loader(): void; export declare const action: () => void; export declare class ServerOnly {} export const clientLoader = () => null; - `, - '/app/routes/ambient-exports.tsx' - ); - const exportInfo = { - exportNames: analysis.exportNames, - exportAllModules: analysis.exportAllModules, - }; - - expect(exportInfo).toEqual({ + `) + ).resolves.toEqual({ exportNames: ['clientLoader'], exportAllModules: [], }); - await expect(getExportNamesAndExportAll(analysis.code)).resolves.toEqual( - exportInfo - ); - }); -}); - -describe('transformToEsm', () => { - it('preserves arrow function object return parentheses', async () => { - const code = ` - const items = [{ pathname: '/', data: 'Home' }]; - export const labels = items.map((item) => ({ - to: item.pathname, - label: item.data, - })); - `; - - const transformed = await transformToEsm(code, 'route.tsx'); - - expect(transformed).toContain('=> ({'); - expect(() => parse(transformed, { sourceType: 'module' })).not.toThrow(); }); }); diff --git a/tests/index.test.ts b/tests/index.test.ts index 85db3fe..5dc1b3d 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -40,6 +40,12 @@ describe('pluginReactRouter', () => { paths: 'custom.config.ts', type: 'reload-server', }, + { + paths: expect.stringMatching( + /react-router\.config\.[cm]?[jt]sx?$/ + ), + type: 'reload-server', + }, { paths: expect.stringMatching(/app\/routes\.[cm]?[jt]sx?$/), type: 'reload-server', @@ -54,49 +60,40 @@ describe('pluginReactRouter', () => { ); }); - it('emits the route restart marker as a web build asset', async () => { - const rsbuild = await createStubRsbuild({ - action: 'build', - rsbuildConfig: {}, - }); - - rsbuild.addPlugins([pluginReactRouter()]); - await rsbuild.unwrapConfig(); - - const processAssetsCall = rsbuild.processAssets.mock.calls.find( - ([options]) => - options.stage === 'additional' && options.targets?.includes('web') - ); - expect(processAssetsCall).toBeDefined(); - - const handler = processAssetsCall?.[1]; - const emitAsset = rstest.fn(); - const updateAsset = rstest.fn(); - const RawSource = class { - constructor(private readonly content: string) {} - source() { - return this.content; - } - size() { - return this.content.length; - } + it('watches all supported config filenames when the config does not exist yet', async () => { + const existsSyncMock = fs.existsSync as unknown as { + mockImplementation: (implementation: (path: unknown) => boolean) => void; + mockReturnValue: (value: boolean) => void; }; + existsSyncMock.mockImplementation( + path => !String(path).includes('react-router.config') + ); - handler({ - sources: { RawSource }, - compilation: { - getAsset: rstest.fn().mockReturnValue(undefined), - emitAsset, - updateAsset, - }, - }); + try { + const rsbuild = await createStubRsbuild({ + rsbuildConfig: {}, + }); - expect(emitAsset).toHaveBeenCalledWith( - '.react-router/route-watch', - expect.any(RawSource) - ); - expect(emitAsset.mock.calls[0][1].source()).not.toBe(''); - expect(updateAsset).not.toHaveBeenCalled(); + rsbuild.addPlugins([pluginReactRouter()]); + const config = await rsbuild.unwrapConfig(); + const configWatch = config.dev.watchFiles.find( + (watchFile: { paths: unknown }) => Array.isArray(watchFile.paths) + ); + + expect(configWatch).toMatchObject({ + paths: expect.arrayContaining([ + expect.stringMatching(/react-router\.config\.tsx$/), + expect.stringMatching(/react-router\.config\.ts$/), + expect.stringMatching(/react-router\.config\.jsx$/), + expect.stringMatching(/react-router\.config\.js$/), + expect.stringMatching(/react-router\.config\.mjs$/), + expect.stringMatching(/react-router\.config\.mts$/), + ]), + type: 'reload-server', + }); + } finally { + existsSyncMock.mockReturnValue(true); + } }); it('should respect server output format', async () => { diff --git a/tests/manifest.test.ts b/tests/manifest.test.ts index 2d77c1e..301ff33 100644 --- a/tests/manifest.test.ts +++ b/tests/manifest.test.ts @@ -5,9 +5,9 @@ import { describe, expect, it } from '@rstest/core'; import { createReactRouterManifestStats, configRoutesToRouteManifest, + generateReactRouterManifestForDev, getReactRouterManifestForDev, getReactRouterManifestChunkNames, - getRouteManifestModuleExports, } from '../src/manifest'; const createTempApp = (routeCode: string) => { @@ -332,25 +332,26 @@ describe('manifest', () => { export default function Page() { return null; } `); try { - const manifest = await getReactRouterManifestForDev( - routes, - {}, - clientStats, - appDir, - '/', - { - isBuild: true, - rootRouteFile: 'root.tsx', - splitRouteModules: false, - } - ); + const { manifest, moduleExportsByRouteId } = + await generateReactRouterManifestForDev( + routes, + {}, + clientStats, + appDir, + '/', + { + isBuild: true, + rootRouteFile: 'root.tsx', + splitRouteModules: false, + } + ); const routeManifest = manifest.routes['routes/page']; expect(routeManifest).toMatchObject({ hasAction: true, hasLoader: true, }); - expect(getRouteManifestModuleExports(manifest)['routes/page']).toEqual( + expect(moduleExportsByRouteId['routes/page']).toEqual( expect.arrayContaining(['headers', 'action', 'loader', 'default']) ); expect(routeManifest).not.toHaveProperty('headers'); diff --git a/tests/modify-browser-manifest.test.ts b/tests/modify-browser-manifest.test.ts index 4141abf..34f278e 100644 --- a/tests/modify-browser-manifest.test.ts +++ b/tests/modify-browser-manifest.test.ts @@ -25,6 +25,46 @@ const createAsset = (source: string) => ({ }); describe('modify browser manifest plugin', () => { + it('rejects the promise hook when build route analysis fails', async () => { + const { root, appDir } = createTempApp(); + writeFileSync(join(appDir, 'routes/page.tsx'), 'export const = broken;'); + const routes = { + root: { id: 'root', file: 'root.tsx', path: '' }, + 'routes/page': { + id: 'routes/page', + parentId: 'root', + file: 'routes/page.tsx', + path: 'page', + }, + }; + let emit: ((compilation: unknown) => Promise) | undefined; + const compiler = { + hooks: { + emit: { + tapPromise(_name: string, callback: typeof emit) { + emit = callback; + }, + }, + }, + }; + + try { + createModifyBrowserManifestPlugin(routes, {}, appDir, '/', { + isBuild: true, + }).apply(compiler as never); + + expect(emit).toBeDefined(); + await expect( + emit?.({ + namedChunks: new Map(), + assets: {}, + }) + ).rejects.toThrow(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + it('does not read ignored chunk files while creating manifest stats', async () => { const { root, appDir } = createTempApp(); const routes = { @@ -36,13 +76,11 @@ describe('modify browser manifest plugin', () => { path: 'page', }, }; - let emit: - | ((compilation: unknown, callback: (error?: Error) => void) => void) - | undefined; + let emit: ((compilation: unknown) => Promise) | undefined; const compiler = { hooks: { emit: { - tapAsync(_name: string, callback: typeof emit) { + tapPromise(_name: string, callback: typeof emit) { emit = callback; }, }, @@ -61,35 +99,18 @@ describe('modify browser manifest plugin', () => { }, }); - await new Promise((resolve, reject) => { - emit?.( - { - namedChunks: new Map([ - [ - 'entry.client', - { files: new Set(['static/js/entry.client.js']) }, - ], - ['root', { files: new Set(['static/js/root.js']) }], - [ - 'routes/page', - { files: new Set(['static/js/routes/page.js']) }, - ], - ['vendor', ignoredChunk], - ]), - assets: { - 'static/js/virtual/react-router/browser-manifest.js': createAsset( - 'window.__reactRouterManifest="PLACEHOLDER";' - ), - }, - }, - error => { - if (error) { - reject(error); - return; - } - resolve(); - } - ); + await emit?.({ + namedChunks: new Map([ + ['entry.client', { files: new Set(['static/js/entry.client.js']) }], + ['root', { files: new Set(['static/js/root.js']) }], + ['routes/page', { files: new Set(['static/js/routes/page.js']) }], + ['vendor', ignoredChunk], + ]), + assets: { + 'static/js/virtual/react-router/browser-manifest.js': createAsset( + 'window.__reactRouterManifest="PLACEHOLDER";' + ), + }, }); } finally { rmSync(root, { recursive: true, force: true }); @@ -107,13 +128,11 @@ describe('modify browser manifest plugin', () => { path: 'page', }, }; - let emit: - | ((compilation: unknown, callback: (error?: Error) => void) => void) - | undefined; + let emit: ((compilation: unknown) => Promise) | undefined; const compiler = { hooks: { emit: { - tapAsync(_name: string, callback: typeof emit) { + tapPromise(_name: string, callback: typeof emit) { emit = callback; }, }, @@ -132,11 +151,7 @@ describe('modify browser manifest plugin', () => { isBuild: true, }, { - manifestChunkNames: new Set([ - 'entry.client', - 'root', - 'routes/page', - ]), + manifestChunkNames: new Set(['entry.client', 'root', 'routes/page']), } ).apply(compiler as never); @@ -147,35 +162,18 @@ describe('modify browser manifest plugin', () => { }, }); - await new Promise((resolve, reject) => { - emit?.( - { - namedChunks: new Map([ - [ - 'entry.client', - { files: new Set(['static/js/entry.client.js']) }, - ], - ['root', { files: new Set(['static/js/root.js']) }], - [ - 'routes/page', - { files: new Set(['static/js/routes/page.js']) }, - ], - ['routes/page-client-loader', theoreticalSplitChunk], - ]), - assets: { - 'static/js/virtual/react-router/browser-manifest.js': createAsset( - 'window.__reactRouterManifest="PLACEHOLDER";' - ), - }, - }, - error => { - if (error) { - reject(error); - return; - } - resolve(); - } - ); + await emit?.({ + namedChunks: new Map([ + ['entry.client', { files: new Set(['static/js/entry.client.js']) }], + ['root', { files: new Set(['static/js/root.js']) }], + ['routes/page', { files: new Set(['static/js/routes/page.js']) }], + ['routes/page-client-loader', theoreticalSplitChunk], + ]), + assets: { + 'static/js/virtual/react-router/browser-manifest.js': createAsset( + 'window.__reactRouterManifest="PLACEHOLDER";' + ), + }, }); } finally { rmSync(root, { recursive: true, force: true }); diff --git a/tests/parallel-route-transforms.test.ts b/tests/parallel-route-transforms.test.ts index 6120941..72539a5 100644 --- a/tests/parallel-route-transforms.test.ts +++ b/tests/parallel-route-transforms.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, it, rstest } from '@rstest/core'; -import * as exportUtils from '../src/export-utils'; +import { describe, expect, it } from '@rstest/core'; +import { getExportNames } from '../src/export-utils'; import { executeRouteTransformTask, type RouteModuleTransformTask, @@ -7,6 +7,7 @@ import { import { createRouteTransformExecutor, getDefaultWorkerCount, + shouldParallelizeRouteTransforms, } from '../src/parallel-route-transforms'; import type { RouteChunkConfig } from '../src/route-chunks'; @@ -34,6 +35,7 @@ const createRouteModuleTask = ( resource: `${resourcePath}?react-router-route`, resourcePath, environmentName: 'web', + sourceMaps: true, ssr: true, isBuild: false, isSpaMode: false, @@ -42,6 +44,15 @@ const createRouteModuleTask = ( }); describe('parallel route transforms', () => { + it.each([ + [48, false], + [255, false], + [256, true], + [1024, true], + ])('selects the adaptive default for %i routes', (routeCount, expected) => { + expect(shouldParallelizeRouteTransforms(routeCount)).toBe(expected); + }); + it.each([ [1, 0], [2, 0], @@ -50,12 +61,26 @@ describe('parallel route transforms', () => { [6, 4], [8, 6], [10, 8], - [12, 10], - [24, 22], - ])('defaults to cpu count minus two workers', (cpus, workers) => { + [12, 8], + [24, 8], + ])('caps the automatic worker count', (cpus, workers) => { expect(getDefaultWorkerCount(cpus)).toBe(workers); }); + it('rejects unsafe explicit worker counts', () => { + expect(() => + createRouteTransformExecutor({ + parallelTransforms: { maxWorkers: 1.5 }, + }) + ).toThrow('must be a positive integer'); + + expect(() => + createRouteTransformExecutor({ + parallelTransforms: { maxWorkers: 33 }, + }) + ).toThrow('must not exceed 32'); + }); + it('honors explicit maxWorkers', async () => { const executor = createRouteTransformExecutor({ parallelTransforms: { maxWorkers: 2 }, @@ -105,107 +130,6 @@ describe('parallel route transforms', () => { }); }); - it('does not run bundler route analysis for client entries without split route chunks', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - await executeRouteTransformTask({ - kind: 'routeClientEntry', - code: ` - export async function loader() { return null; } - export async function clientLoader() { return null; } - export default function Route() { return null; } - `, - resourcePath, - environmentName: 'web', - isBuild: false, - routeChunkConfig: disabledRouteChunkConfig, - }); - - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - - it('does not run bundler route analysis for split client entries without split export names', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - const result = await executeRouteTransformTask({ - kind: 'routeClientEntry', - code: ` - export async function loader() { return null; } - export default function Route() { return null; } - `, - resourcePath, - environmentName: 'web', - isBuild: true, - routeChunkConfig, - }); - - expect(result.code).toBe( - `export { default } from "${resourcePath}?react-router-route";` - ); - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - - it('does not run bundler route analysis for split route export modules without split export names', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - const code = ` - export async function loader() { return null; } - export default function Route() { return null; } - `; - - try { - const result = await executeRouteTransformTask({ - kind: 'splitRouteExports', - code, - resourcePath, - routeChunkConfig, - }); - - expect(result).toEqual({ code, map: null }); - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - - it('does not run bundler route analysis for client-only stubs', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - await executeRouteTransformTask({ - kind: 'clientOnlyStub', - code: ` - export const clientValue = 'client'; - export default function ClientOnly() { return null; } - `, - resourcePath: '/app/client-data.client.ts', - }); - - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - it('can execute route module tasks through worker-backed parallelism', async () => { const executor = createRouteTransformExecutor({ parallelTransforms: { maxWorkers: 2 }, @@ -221,7 +145,7 @@ describe('parallel route transforms', () => { } }); - it('shares build route module results across environments when output is identical', async () => { + it('produces identical build route modules when environments need the same output', async () => { const executor = createRouteTransformExecutor({ parallelTransforms: { maxWorkers: 2 }, splitRouteModules: true, @@ -248,12 +172,45 @@ describe('parallel route transforms', () => { } }); - it('does not share build route module results when web removes server-only exports', async () => { + it('keeps environment-specific build route module output isolated', async () => { + const executor = createRouteTransformExecutor({ + parallelTransforms: { maxWorkers: 2 }, + splitRouteModules: true, + }); + const task = createRouteModuleTask({ + environmentName: 'node', + isBuild: true, + }); + + try { + const nodeResult = await executor.run(task); + const webResult = await executor.run({ + ...task, + environmentName: 'web', + }); + + await expect(getExportNames(nodeResult.code)).resolves.toContain( + 'loader' + ); + await expect(getExportNames(webResult.code)).resolves.not.toContain( + 'loader' + ); + } finally { + await executor.close(); + } + }); + + it('isolates escaped server exports across build environments', async () => { const executor = createRouteTransformExecutor({ parallelTransforms: { maxWorkers: 2 }, splitRouteModules: true, }); const task = createRouteModuleTask({ + code: String.raw` + const implementation = async () => null; + export { implementation as lo\u0061der }; + export default function Route() { return null; } + `, environmentName: 'node', isBuild: true, }); @@ -272,6 +229,22 @@ describe('parallel route transforms', () => { } }); + it('preserves runtime TypeScript for the downstream Rsbuild SWC stage', async () => { + const result = await executeRouteTransformTask( + createRouteModuleTask({ + code: ` + export enum Status { Active } + export default function Route() { return Status.Active; } + `, + environmentName: 'node', + isBuild: true, + }) + ); + + expect(result.code).toContain('enum Status'); + expect(result.code).toContain('Status.Active'); + }); + it('preserves value imports when web route modules have no server-only exports', async () => { const result = await executeRouteTransformTask( createRouteModuleTask({ @@ -288,46 +261,6 @@ describe('parallel route transforms', () => { expect(result.code).toContain(`import { setup } from './side-effect';`); }); - it('does not run bundler route analysis for non-SPA route module transforms', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - await executeRouteTransformTask(createRouteModuleTask()); - - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - - it('validates SPA route modules without bundler route analysis', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - const result = await executeRouteTransformTask( - createRouteModuleTask({ - code: ` - export async function clientLoader() { return null; } - export default function Route() { return null; } - `, - ssr: false, - isSpaMode: true, - }) - ); - - expect(result.code).toContain('clientLoader'); - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } - }); - it('rejects invalid SPA route module exports from the route transform AST', async () => { await expect( executeRouteTransformTask( @@ -343,7 +276,7 @@ describe('parallel route transforms', () => { ).rejects.toThrow('SPA Mode: 1 invalid route export'); }); - it('generates route module source maps only outside build mode', async () => { + it('generates route module source maps when the environment requests them', async () => { const task = createRouteModuleTask({ code: ` export async function loader() { return null; } @@ -351,12 +284,11 @@ describe('parallel route transforms', () => { `, }); - await expect( - executeRouteTransformTask({ - ...task, - isBuild: true, - }) - ).resolves.toMatchObject({ map: null }); + const buildResult = await executeRouteTransformTask({ + ...task, + isBuild: true, + }); + expect(buildResult.map).not.toBeNull(); const devResult = await executeRouteTransformTask({ ...task, @@ -364,5 +296,11 @@ describe('parallel route transforms', () => { }); expect(devResult.map).not.toBeNull(); + + const withoutSourceMaps = await executeRouteTransformTask({ + ...task, + sourceMaps: false, + }); + expect(withoutSourceMaps.map).toBeNull(); }); }); diff --git a/tests/plugin-utils.test.ts b/tests/plugin-utils.test.ts index ca85de0..673ebd7 100644 --- a/tests/plugin-utils.test.ts +++ b/tests/plugin-utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from '@rstest/core'; -import { generate, parse } from '../src/babel'; +import { generate, parse } from '../src/yuku'; import { combineURLs, stripFileExtension, @@ -131,13 +131,35 @@ describe('plugin-utils', () => { }); describe('transformRoute', () => { + it('preserves bundler directives while transforming routes', () => { + const result = transformRouteCode(` + export default function Route() { + return import(/* webpackChunkName: "route-data" */ './data'); + } + `); + + expect(result).toContain('webpackChunkName'); + }); + + it('preserves named default class bindings', () => { + const result = transformRouteCode(` + export default class Route {} + Route.displayName = 'Route'; + `); + + expect(result).toMatch(/class Route/); + expect(result).toMatch(/export default _withComponentProps\(Route\)/); + expect(result).toContain(`Route.displayName = 'Route'`); + }); + it('wraps default class exports with component props', () => { const result = transformRouteCode(` export default class Route {} `); expect(result).toContain('withComponentProps'); - expect(result).toMatch(/export default _withComponentProps\(class Route/); + expect(result).toMatch(/class Route/); + expect(result).toMatch(/export default _withComponentProps\(Route\)/); }); it('wraps named class component exports', () => { @@ -166,6 +188,43 @@ describe('plugin-utils', () => { expect(result).toMatch(/export \{ _ErrorBoundary as ErrorBoundary \}/); }); + it('wraps component exports re-exported from another module', () => { + const result = transformRouteCode(` + export { Boundary as ErrorBoundary } from './boundary'; + `); + + expect(result).toMatch( + /import \{ Boundary as _ErrorBoundarySource \} from ["']\.\/boundary["']/ + ); + expect(result).toMatch( + /const _ErrorBoundary = _withErrorBoundaryProps\(_ErrorBoundarySource\)/ + ); + expect(result).toMatch(/export \{ _ErrorBoundary as ErrorBoundary \}/); + }); + + it('preserves side-effect import order before wrapped source re-exports', () => { + const result = transformRouteCode(` + import './setup'; + export { Boundary as ErrorBoundary } from './boundary'; + `); + + expect(result.indexOf("import './setup'")).toBeLessThan( + result.search( + /import\s*\{\s*Boundary as _ErrorBoundarySource\s*\}\s*from ['"]\.\/boundary['"]/ + ) + ); + }); + + it('does not turn type-only exports into runtime component wrappers', () => { + const result = transformRouteCode(` + type Boundary = { message: string }; + export type { Boundary as ErrorBoundary }; + `); + + expect(result).not.toContain('withErrorBoundaryProps'); + expect(result).not.toContain('const _ErrorBoundary'); + }); + it('avoids top-level generated helper name collisions', () => { const result = transformRouteCode(` const _withComponentProps = 'reserved'; @@ -185,7 +244,8 @@ describe('plugin-utils', () => { `); expect(result).toContain('withComponentProps as _withComponentProps'); - expect(result).toContain('export default _withComponentProps(function Route'); + expect(result).toContain('function Route'); + expect(result).toContain('export default _withComponentProps(Route)'); expect(result).not.toContain('_withComponentProps2'); }); @@ -200,5 +260,4 @@ describe('plugin-utils', () => { expect(result).not.toContain('_withComponentProps2'); }); }); - }); diff --git a/tests/prerender.test.ts b/tests/prerender.test.ts index 2f3171a..17bc814 100644 --- a/tests/prerender.test.ts +++ b/tests/prerender.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from '@rstest/core'; -import { getPrerenderConcurrency, getStaticPrerenderPaths, resolvePrerenderPaths } from '../src/prerender'; +import { + getPrerenderConcurrency, + getStaticPrerenderPaths, + resolvePrerenderPaths, +} from '../src/prerender'; import type { RouteConfigEntry } from '@react-router/dev/routes'; const routes: RouteConfigEntry[] = [ @@ -87,8 +91,8 @@ describe('prerender helpers', () => { expect( getPrerenderConcurrency({ paths: ['/'], unstable_concurrency: 3 }) ).toBe(3); - expect(getPrerenderConcurrency({ paths: ['/'] }, 24)).toBe(22); + expect(getPrerenderConcurrency({ paths: ['/'] }, 24)).toBe(1); expect(getPrerenderConcurrency({ paths: ['/'] }, 3)).toBe(1); - expect(getPrerenderConcurrency({ paths: ['/'] }, 2)).toBe(0); + expect(getPrerenderConcurrency({ paths: ['/'] }, 2)).toBe(1); }); }); diff --git a/tests/remove-exports.test.ts b/tests/remove-exports.test.ts index 144a597..6e690da 100644 --- a/tests/remove-exports.test.ts +++ b/tests/remove-exports.test.ts @@ -1,23 +1,7 @@ import { describe, expect, it } from '@rstest/core'; -import { generate, parse, traverse } from '../src/babel'; +import { generate, parse } from '../src/yuku'; import { removeExports, removeUnusedImports } from '../src/plugin-utils'; -function hasTopLevelAssignment(ast: any, textIncludes: string): boolean { - let found = false; - traverse(ast, { - ExpressionStatement(path) { - if (!path.parentPath.isProgram()) return; - const expr = path.node.expression; - if (expr.type !== 'AssignmentExpression') return; - const raw = path.toString(); - if (raw.includes(textIncludes)) { - found = true; - } - }, - }); - return found; -} - describe('removeExports', () => { it('returns false when no matching export can be removed', () => { const code = ` @@ -63,7 +47,7 @@ describe('removeExports', () => { removeExports(ast, ['loader']); // The export specifier should be gone and the assignment too. - expect(hasTopLevelAssignment(ast, 'local.hydrate')).toBe(false); + expect(generate(ast).code).not.toContain('local.hydrate'); }); it('removes top-level property assignment when default export is removed', () => { @@ -76,7 +60,7 @@ describe('removeExports', () => { const ast = parse(code, { sourceType: 'module' }); removeExports(ast, ['default']); - expect(hasTopLevelAssignment(ast, 'Root.displayName')).toBe(false); + expect(generate(ast).code).not.toContain('Root.displayName'); }); it('removes unused imports after removing server-only exports', () => { @@ -94,19 +78,10 @@ describe('removeExports', () => { removeExports(ast, ['action']); removeUnusedImports(ast); - let hasThemeImport = false; - traverse(ast, { - ImportDeclaration(path) { - if (path.node.source.value === './theme.server') { - hasThemeImport = true; - } - }, - }); - - expect(hasThemeImport).toBe(false); + expect(generate(ast).code).not.toContain('./theme.server'); }); - it('removes export-all declarations when removing server-only exports', () => { + it('preserves export-all declarations that cannot be filtered safely', () => { const code = ` export * from './data.server'; export default function Route() { @@ -118,10 +93,28 @@ describe('removeExports', () => { removeExports(ast, ['loader']); const result = generate(ast).code; - expect(result).not.toContain("export * from './data.server'"); + expect(result).toContain("export * from './data.server'"); expect(result).toContain('Route'); }); + it('keeps lowercase JSX member imports after removing server exports', () => { + const code = ` + import { motion } from 'framer-motion'; + export async function loader() { return null; } + export default function Route() { + return ; + } + `; + + const ast = parse(code, { sourceType: 'module' }); + removeExports(ast, ['loader']); + removeUnusedImports(ast); + + const result = generate(ast).code; + expect(result).toContain("import { motion } from 'framer-motion'"); + expect(result).toContain(' { const code = ` import { @@ -207,8 +200,7 @@ describe('removeExports', () => { it('removes every declaration in a deep dead dependency chain', () => { const helperCount = 64; const helpers = Array.from({ length: helperCount }, (_, index) => { - const value = - index === helperCount - 1 ? '1' : `helper${index + 1}()`; + const value = index === helperCount - 1 ? '1' : `helper${index + 1}()`; return `const helper${index} = () => ${value};`; }).join('\n'); const code = ` diff --git a/tests/route-artifacts.test.ts b/tests/route-artifacts.test.ts index 8200d6b..d9dbd0f 100644 --- a/tests/route-artifacts.test.ts +++ b/tests/route-artifacts.test.ts @@ -1,5 +1,4 @@ -import { describe, expect, it, rstest } from '@rstest/core'; -import * as exportUtils from '../src/export-utils'; +import { describe, expect, it } from '@rstest/core'; import { createRouteChunkArtifact, createRouteClientEntryArtifact, @@ -96,72 +95,56 @@ describe('route artifact helpers', () => { }); it('excludes split client exports from web build route entries', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); - - try { - const result = await createRouteClientEntryArtifact({ - code: ` + const result = await createRouteClientEntryArtifact({ + code: ` export const clientAction = async () => {}; export async function clientLoader() { return null; } export default function Route() { return null; } `, - resourcePath, - environmentName: 'web', - isBuild: true, - routeChunkConfig, - }); - - expect(result).toEqual({ - code: `export { default } from ${JSON.stringify(routeRequest)};`, - }); - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } + resourcePath, + environmentName: 'web', + isBuild: true, + routeChunkConfig, + }); + + expect(result).toEqual({ + code: `export { default } from ${JSON.stringify(routeRequest)};`, + }); }); it('does not run split analysis for root route client entries', async () => { - const getBundlerRouteAnalysis = rstest.spyOn( - exportUtils, - 'getBundlerRouteAnalysis' - ); const rootResourcePath = '/app/root.tsx'; - - try { - const result = await createRouteClientEntryArtifact({ - code: ` + const result = await createRouteClientEntryArtifact({ + code: ` export async function clientLoader() { return null; } export function HydrateFallback() { return null; } export default function Root() { return null; } `, - resourcePath: rootResourcePath, - environmentName: 'web', - isBuild: true, - routeChunkConfig, - }); - - expect(result).toEqual({ - code: `export { HydrateFallback, clientLoader, default } from ${JSON.stringify( - `${rootResourcePath}?react-router-route` - )};`, - }); - expect(getBundlerRouteAnalysis).not.toHaveBeenCalled(); - } finally { - getBundlerRouteAnalysis.mockRestore(); - } + resourcePath: rootResourcePath, + environmentName: 'web', + isBuild: true, + routeChunkConfig, + }); + + expect(result).toEqual({ + code: `export { HydrateFallback, clientLoader, default } from ${JSON.stringify( + `${rootResourcePath}?react-router-route` + )};`, + }); }); }); describe('createRouteChunkArtifact', () => { it('returns the disabled split-route empty snippet with a null map', async () => { await expect( - createRouteChunk(`export const clientLoader = async () => {};`, 'clientLoader', { - config: disabledRouteChunkConfig, - isBuild: true, - }) + createRouteChunk( + `export const clientLoader = async () => {};`, + 'clientLoader', + { + config: disabledRouteChunkConfig, + isBuild: true, + } + ) ).resolves.toEqual({ code: emptyRouteChunkSnippet('Split route modules disabled'), map: null, @@ -177,25 +160,23 @@ describe('route artifact helpers', () => { routeChunkConfig, isBuild: true, }) - ).rejects.toThrow(`Invalid route chunk name in "${resourcePath}?route-chunk=invalid"`); + ).rejects.toThrow( + `Invalid route chunk name in "${resourcePath}?route-chunk=invalid"` + ); }); - it('generates the same route chunk code as the existing transformed ESM path', async () => { + it('generates the expected route chunk code from source', async () => { const source = ` export const clientAction = async () => {}; export default function Route() { return null; } `; const cache: RouteChunkCache = new Map(); - const analysis = await exportUtils.getBundlerRouteAnalysis( - source, - resourcePath - ); const expectedCode = await getRouteChunkIfEnabled( cache, routeChunkConfig, resourcePath, 'clientAction', - analysis.code + source ); const result = await createRouteChunk(source, 'clientAction', { cache }); diff --git a/tests/route-watch.test.ts b/tests/route-watch.test.ts index 0784ef3..a9740e4 100644 --- a/tests/route-watch.test.ts +++ b/tests/route-watch.test.ts @@ -8,14 +8,108 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { describe, expect, it } from '@rstest/core'; +import { describe, expect, it, rstest } from '@rstest/core'; import { createRouteManifestSnapshot, + createRouteTopologyWatcher, ensureDevRestartMarker, getRouteRestartMarkerPath, } from '../src/route-watch'; describe('route watch restart marker', () => { + it('allows a topology callback to await watcher shutdown', async () => { + const root = mkdtempSync(join(tmpdir(), 'rr-route-watch-')); + const markerPath = join(root, 'build/.react-router-route-watch'); + const watchedDirectory = join(root, 'app'); + mkdirSync(watchedDirectory, { recursive: true }); + let topology = new Set(['initial']); + let triggerChange!: () => void; + let close!: () => Promise; + let callbackCompleted = false; + + try { + close = await createRouteTopologyWatcher({ + watchDirectory: watchedDirectory, + restartMarkerPath: markerPath, + getRouteTopology: async () => topology, + onRouteTopologyChange: async () => { + await close(); + callbackCompleted = true; + }, + onError: error => { + throw error; + }, + watchDirectoryEntry: (_directory, onChange) => { + triggerChange = onChange; + return { close: () => {} }; + }, + }); + + topology = new Set(['changed']); + triggerChange(); + + await expect.poll(() => callbackCompleted, { timeout: 2000 }).toBe(true); + } finally { + await close?.(); + rmSync(root, { recursive: true, force: true }); + } + }); + + it('does not recreate watchers or touch the marker after close', async () => { + const root = mkdtempSync(join(tmpdir(), 'rr-route-watch-')); + const markerPath = join(root, 'build/.react-router-route-watch'); + const watchedDirectory = join(root, 'app'); + mkdirSync(watchedDirectory, { recursive: true }); + await ensureDevRestartMarker(markerPath); + const initialMarker = readFileSync(markerPath, 'utf8'); + let topologyReads = 0; + let releaseRescan!: () => void; + const rescanReleased = new Promise(resolve => { + releaseRescan = resolve; + }); + let markRescanStarted!: () => void; + const rescanStarted = new Promise(resolve => { + markRescanStarted = resolve; + }); + let triggerChange!: () => void; + const closeWatcher = rstest.fn(); + + try { + const close = await createRouteTopologyWatcher({ + watchDirectory: watchedDirectory, + restartMarkerPath: markerPath, + onError: error => { + throw error; + }, + getRouteTopology: async () => { + topologyReads += 1; + if (topologyReads === 1) { + return new Set(['initial']); + } + markRescanStarted(); + await rescanReleased; + return new Set(['changed']); + }, + watchDirectoryEntry: (_directory, onChange) => { + triggerChange = onChange; + return { close: closeWatcher }; + }, + }); + + triggerChange(); + await rescanStarted; + const closePromise = close(); + releaseRescan(); + await closePromise; + + expect(readFileSync(markerPath, 'utf8')).toBe(initialMarker); + expect(closeWatcher).toHaveBeenCalled(); + } finally { + releaseRescan(); + rmSync(root, { recursive: true, force: true }); + } + }); + it('places the restart marker in the client build output', () => { expect(getRouteRestartMarkerPath('/project/build/client')).toBe( '/project/build/client/.react-router/route-watch' diff --git a/tests/warn-on-client-source-maps.test.ts b/tests/warn-on-client-source-maps.test.ts index bf385bc..d9a83a6 100644 --- a/tests/warn-on-client-source-maps.test.ts +++ b/tests/warn-on-client-source-maps.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it, rstest } from '@rstest/core'; -import { warnOnClientSourceMaps } from '../src/warnings/warn-on-client-source-maps'; +import { + isSourceMapEnabled, + warnOnClientSourceMaps, +} from '../src/warnings/warn-on-client-source-maps'; describe('warnOnClientSourceMaps', () => { it('does not warn in non-production mode', () => { @@ -57,6 +60,11 @@ describe('warnOnClientSourceMaps', () => { expect(warn).toHaveBeenCalledTimes(1); }); + it('treats string output.sourceMap values as enabled', () => { + expect(isSourceMapEnabled('source-map')).toBe(true); + expect(isSourceMapEnabled('hidden-source-map')).toBe(true); + }); + it('does not warn when source maps are disabled in production', () => { const warn = rstest.fn(); warnOnClientSourceMaps(