From 9b5cabb065e6766e7c07cd87400d18df16b7fe99 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Tue, 28 May 2024 12:03:47 -0400 Subject: [PATCH 1/8] defineRoute$ macro transforms --- packages/remix-dev/vite/dce.ts | 285 ++++++++++++++++++++++++ packages/remix-dev/vite/define-route.ts | 110 +++++++++ 2 files changed, 395 insertions(+) create mode 100644 packages/remix-dev/vite/dce.ts create mode 100644 packages/remix-dev/vite/define-route.ts diff --git a/packages/remix-dev/vite/dce.ts b/packages/remix-dev/vite/dce.ts new file mode 100644 index 0000000000..ec8fac47bd --- /dev/null +++ b/packages/remix-dev/vite/dce.ts @@ -0,0 +1,285 @@ +// Adapted from https://github.com/egoist/babel-plugin-eliminator/blob/d29859396b7708b7f7abbacdd951cbbc80902f00/src/index.ts + +import type { ParseResult } from "@babel/parser"; +import { traverse, t, type NodePath, type BabelTypes } from "./babel"; + +type IdentifierPath = NodePath; + +export function findReferencedIdentifiers( + ast: ParseResult +): Set { + const refs = new Set(); + + function markFunction( + path: NodePath< + | BabelTypes.FunctionDeclaration + | BabelTypes.FunctionExpression + | BabelTypes.ArrowFunctionExpression + > + ) { + const ident = getIdentifier(path); + if (ident?.node && isIdentifierReferenced(ident)) { + refs.add(ident); + } + } + + function markImport( + path: NodePath< + | BabelTypes.ImportSpecifier + | BabelTypes.ImportDefaultSpecifier + | BabelTypes.ImportNamespaceSpecifier + > + ) { + const local = path.get("local"); + if (isIdentifierReferenced(local)) { + refs.add(local); + } + } + + traverse(ast, { + VariableDeclarator(path) { + if (path.node.id.type === "Identifier") { + const local = path.get("id") as NodePath; + if (isIdentifierReferenced(local)) { + refs.add(local); + } + } else if (path.node.id.type === "ObjectPattern") { + const pattern = path.get("id") as NodePath; + + const properties = pattern.get("properties"); + properties.forEach((p) => { + const local = p.get( + p.node.type === "ObjectProperty" + ? "value" + : p.node.type === "RestElement" + ? "argument" + : (function () { + throw new Error("invariant"); + })() + ) as NodePath; + if (isIdentifierReferenced(local)) { + refs.add(local); + } + }); + } else if (path.node.id.type === "ArrayPattern") { + const pattern = path.get("id") as NodePath; + + const elements = pattern.get("elements"); + elements.forEach((e) => { + let local: NodePath; + if (e.node?.type === "Identifier") { + local = e as NodePath; + } else if (e.node?.type === "RestElement") { + local = e.get("argument") as NodePath; + } else { + return; + } + + if (isIdentifierReferenced(local)) { + refs.add(local); + } + }); + } + }, + + FunctionDeclaration: markFunction, + FunctionExpression: markFunction, + ArrowFunctionExpression: markFunction, + ImportSpecifier: markImport, + ImportDefaultSpecifier: markImport, + ImportNamespaceSpecifier: markImport, + }); + return refs; +} + +/** + * @param refs - If provided, only these identifiers will be considered for removal. + */ +export const deadCodeElimination = ( + ast: ParseResult, + refs?: Set +) => { + let referencesRemovedInThisPass: number; + + let shouldBeRemoved = (ident: IdentifierPath) => { + if (isIdentifierReferenced(ident)) return false; + if (!refs) return true; + return refs.has(ident); + }; + + let sweepFunction = ( + path: NodePath< + | BabelTypes.FunctionDeclaration + | BabelTypes.FunctionExpression + | BabelTypes.ArrowFunctionExpression + > + ) => { + let identifier = getIdentifier(path); + if (identifier?.node && shouldBeRemoved(identifier)) { + ++referencesRemovedInThisPass; + + if ( + t.isAssignmentExpression(path.parentPath.node) || + t.isVariableDeclarator(path.parentPath.node) + ) { + path.parentPath.remove(); + } else { + path.remove(); + } + } + }; + + let sweepImport = ( + path: NodePath< + | BabelTypes.ImportSpecifier + | BabelTypes.ImportDefaultSpecifier + | BabelTypes.ImportNamespaceSpecifier + > + ) => { + let local = path.get("local"); + if (shouldBeRemoved(local)) { + ++referencesRemovedInThisPass; + path.remove(); + if ( + (path.parent as BabelTypes.ImportDeclaration).specifiers.length === 0 + ) { + path.parentPath.remove(); + } + } + }; + + // Traverse again to remove unused references. This happens at least once, + // then repeats until no more references are removed. + do { + referencesRemovedInThisPass = 0; + + traverse(ast, { + Program(path) { + path.scope.crawl(); + }, + // eslint-disable-next-line no-loop-func + VariableDeclarator(path) { + if (path.node.id.type === "Identifier") { + let local = path.get("id") as NodePath; + if (shouldBeRemoved(local)) { + ++referencesRemovedInThisPass; + path.remove(); + } + } else if (path.node.id.type === "ObjectPattern") { + let pattern = path.get("id") as NodePath; + + let beforeCount = referencesRemovedInThisPass; + let properties = pattern.get("properties"); + properties.forEach((property) => { + let local = property.get( + property.node.type === "ObjectProperty" + ? "value" + : property.node.type === "RestElement" + ? "argument" + : (function () { + throw new Error("invariant"); + })() + ) as NodePath; + + if (shouldBeRemoved(local)) { + ++referencesRemovedInThisPass; + property.remove(); + } + }); + + if ( + beforeCount !== referencesRemovedInThisPass && + pattern.get("properties").length < 1 + ) { + path.remove(); + } + } else if (path.node.id.type === "ArrayPattern") { + let pattern = path.get("id") as NodePath; + + let beforeCount = referencesRemovedInThisPass; + let elements = pattern.get("elements"); + elements.forEach((e) => { + let local: NodePath; + if (e.node?.type === "Identifier") { + local = e as NodePath; + } else if (e.node?.type === "RestElement") { + local = e.get("argument") as NodePath; + } else { + return; + } + + if (shouldBeRemoved(local)) { + ++referencesRemovedInThisPass; + e.remove(); + } + }); + + if ( + beforeCount !== referencesRemovedInThisPass && + pattern.get("elements").length < 1 + ) { + path.remove(); + } + } + }, + FunctionDeclaration: sweepFunction, + FunctionExpression: sweepFunction, + ArrowFunctionExpression: sweepFunction, + ImportSpecifier: sweepImport, + ImportDefaultSpecifier: sweepImport, + ImportNamespaceSpecifier: sweepImport, + }); + } while (referencesRemovedInThisPass); +}; + +function getIdentifier( + path: NodePath< + | BabelTypes.FunctionDeclaration + | BabelTypes.FunctionExpression + | BabelTypes.ArrowFunctionExpression + > +): NodePath | null { + let parentPath = path.parentPath; + if (parentPath.type === "VariableDeclarator") { + let variablePath = parentPath as NodePath; + let name = variablePath.get("id"); + return name.node.type === "Identifier" + ? (name as NodePath) + : null; + } + + if (parentPath.type === "AssignmentExpression") { + let variablePath = parentPath as NodePath; + let name = variablePath.get("left"); + return name.node.type === "Identifier" + ? (name as NodePath) + : null; + } + + if (path.node.type === "ArrowFunctionExpression") { + return null; + } + + return path.node.id && path.node.id.type === "Identifier" + ? (path.get("id") as NodePath) + : null; +} + +function isIdentifierReferenced( + ident: NodePath +): boolean { + let binding = ident.scope.getBinding(ident.node.name); + if (binding?.referenced) { + // Functions can reference themselves, so we need to check if there's a + // binding outside the function scope or not. + if (binding.path.type === "FunctionDeclaration") { + return !binding.constantViolations + .concat(binding.referencePaths) + // Check that every reference is contained within the function: + .every((ref) => ref.findParent((parent) => parent === binding?.path)); + } + + return true; + } + return false; +} diff --git a/packages/remix-dev/vite/define-route.ts b/packages/remix-dev/vite/define-route.ts new file mode 100644 index 0000000000..2274fdc7cc --- /dev/null +++ b/packages/remix-dev/vite/define-route.ts @@ -0,0 +1,110 @@ +import type { Binding, NodePath } from "@babel/traverse"; +import { parse, traverse, generate, t } from "./babel"; +import type { GeneratorResult } from "@babel/generator"; +import { deadCodeElimination, findReferencedIdentifiers } from "./dce"; + +// given a route module filepath +// determine if its using `defineRoute$` +// if not, just do the old stuff that looks at exports + +const MACRO = "defineRoute$"; +const MACRO_PKG = "react-router"; +const SERVER_ONLY_PROPERTIES = ["loader", "action"]; + +export function transform( + source: string, + id: string, + options: { ssr: boolean } +): GeneratorResult { + let ast = parse(source, { sourceType: "module" }); + let refs = findReferencedIdentifiers(ast); + + traverse(ast, { + Identifier(path) { + if (t.isImportSpecifier(path.parent)) return; + let binding = path.scope.getBinding(path.node.name); + if (!binding) return; + if (!isMacroBinding(binding)) return; + if (t.isCallExpression(path.parent)) return; + throw path.buildCodeFrameError( + `'${path.node.name}' macro cannot be manipulated at runtime as it must be statically analyzable` + ); + }, + CallExpression(path) { + if (!t.isIdentifier(path.node.callee)) return false; + let macro = path.node.callee.name; + let binding = path.scope.getBinding(macro); + + if (!binding) return false; + if (!isMacroBinding(binding)) return false; + if (path.node.arguments.length !== 1) { + throw path.buildCodeFrameError( + `'${macro}' macro must take exactly one argument` + ); + } + let arg = path.node.arguments[0]; + let argPath = path.get("arguments.0") as NodePath; + if (!t.isObjectExpression(arg)) { + throw argPath.buildCodeFrameError( + `'${macro}' macro argument must be a literal object` + ); + } + + let markedForRemoval: NodePath[] = []; + for (let [i, property] of arg.properties.entries()) { + let propertyPath = argPath.get(`properties.${i}`) as NodePath; + if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { + throw propertyPath.buildCodeFrameError( + `'${macro}' macro argument must only have statically analyzable properties` + ); + } + if (property.computed || !t.isIdentifier(property.key)) { + throw propertyPath.buildCodeFrameError( + `'${macro}' macro argument must only have statically analyzable fields` + ); + } + + if (property.key.name === "params") { + checkRouteParams(propertyPath as NodePath); + } + + if (options.ssr && SERVER_ONLY_PROPERTIES.includes(property.key.name)) { + markedForRemoval.push(propertyPath); + } + } + markedForRemoval.forEach((path) => path.remove()); + }, + }); + deadCodeElimination(ast, refs); + return generate(ast, { sourceMaps: true, sourceFileName: id }, source); +} + +function checkRouteParams(path: NodePath) { + if (t.isObjectMethod(path.node)) { + throw path.buildCodeFrameError(`params must be statically analyzable`); + } + if (!t.isArrayExpression(path.node.value)) { + throw path.buildCodeFrameError(`params must be statically analyzable`); + } + for (let [i, element] of path.node.value.elements.entries()) { + if (!t.isStringLiteral(element)) { + let elementPath = path.get(`value.elements.${i}`) as NodePath; + throw elementPath.buildCodeFrameError( + `params must be statically analyzable` + ); + } + } +} + +function isMacroBinding(binding: Binding): boolean { + // import source + if (!t.isImportDeclaration(binding?.path.parent)) return false; + if (binding.path.parent.source.value !== MACRO_PKG) return false; + + // import specifier + if (!t.isImportSpecifier(binding?.path.node)) return false; + let { imported } = binding.path.node; + if (!t.isIdentifier(imported)) return false; + if (imported.name !== MACRO) return false; + return true; +} From 843025fa2fe96dda9c8375089504032d5402f31e Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Tue, 28 May 2024 12:06:12 -0400 Subject: [PATCH 2/8] defineRoute$ types --- .../react-router/lib/router/define-route.ts | 77 +++++++++++++++++++ packages/react-router/lib/router/utils.ts | 10 +-- 2 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 packages/react-router/lib/router/define-route.ts diff --git a/packages/react-router/lib/router/define-route.ts b/packages/react-router/lib/router/define-route.ts new file mode 100644 index 0000000000..d3e99e669c --- /dev/null +++ b/packages/react-router/lib/router/define-route.ts @@ -0,0 +1,77 @@ +import type { ReactNode } from "react"; + +interface Context {} // TODO: AppLoadContext? + +type MaybePromise = T | Promise; + +type Serializable = + | undefined + | null + | boolean + | string + | symbol + | number + | Array + | { [key: PropertyKey]: Serializable } + | bigint + | Date + | URL + | RegExp + | Error + | Map + | Set + | Promise; + +export type TypedResponse = Omit & { + json(): Promise; +}; + +type DataFunctionReturnValue = + | Serializable + // TODO: | TypedDeferredData> // do we want to allow `defer()` for back compat? + | TypedResponse>; + +// TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) + +export type ResponseStub = { + status: number | undefined; + headers: Headers; +}; + +// loader +type LoaderArgs = { + context: Context; + request: Request; + params: Record; + response: ResponseStub; +}; +export type Loader = ( + args: LoaderArgs +) => MaybePromise; + +// action +type ActionArgs = { + context: Context; + request: Request; + params: Record; + response: ResponseStub; +}; +export type Action = ( + args: ActionArgs +) => MaybePromise; + +type Component

> = (args: { + params: string extends P ? Record : Record; + data: Awaited>; +}) => ReactNode; + +export const defineRoute$ = < + const P extends string, + L extends Loader

, + A extends Action

+>(route: { + params?: P[]; + loader?: L; + action?: A; + component?: Component, NoInfer>; +}) => route; diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index 43f4b5ff6a..4395a5bfc7 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -145,11 +145,11 @@ export type Submission = * Arguments passed to route loader/action functions. Same for now but we keep * this as a private implementation detail in case they diverge in the future. */ -interface DataFunctionArgs { +type DataFunctionArgs = { request: Request; params: Params; context?: Context; -} +}; // TODO: (v7) Change the defaults from any to unknown in and remove Remix wrappers: // ActionFunction, ActionFunctionArgs, LoaderFunction, LoaderFunctionArgs @@ -158,14 +158,12 @@ interface DataFunctionArgs { /** * Arguments passed to loader functions */ -export interface LoaderFunctionArgs - extends DataFunctionArgs {} +export type LoaderFunctionArgs = DataFunctionArgs; /** * Arguments passed to action functions */ -export interface ActionFunctionArgs - extends DataFunctionArgs {} +export type ActionFunctionArgs = DataFunctionArgs; /** * Loaders and actions can return anything except `undefined` (`null` is a From f29d42a85bfcd10b14239eee79ba614f46230e2f Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Wed, 29 May 2024 15:31:41 -0400 Subject: [PATCH 3/8] capitalize Component field in defineRoute, removed json/defer handling --- .../react-router/lib/router/define-route.ts | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/react-router/lib/router/define-route.ts b/packages/react-router/lib/router/define-route.ts index d3e99e669c..57e4f32632 100644 --- a/packages/react-router/lib/router/define-route.ts +++ b/packages/react-router/lib/router/define-route.ts @@ -22,17 +22,6 @@ type Serializable = | Set | Promise; -export type TypedResponse = Omit & { - json(): Promise; -}; - -type DataFunctionReturnValue = - | Serializable - // TODO: | TypedDeferredData> // do we want to allow `defer()` for back compat? - | TypedResponse>; - -// TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) - export type ResponseStub = { status: number | undefined; headers: Headers; @@ -47,7 +36,7 @@ type LoaderArgs = { }; export type Loader = ( args: LoaderArgs -) => MaybePromise; +) => MaybePromise; // action type ActionArgs = { @@ -58,13 +47,16 @@ type ActionArgs = { }; export type Action = ( args: ActionArgs -) => MaybePromise; +) => MaybePromise; type Component

> = (args: { params: string extends P ? Record : Record; data: Awaited>; }) => ReactNode; +// loader -> data for component +// loader -> serverLoader for clientLoader -> data for component +// TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) export const defineRoute$ = < const P extends string, L extends Loader

, @@ -73,5 +65,5 @@ export const defineRoute$ = < params?: P[]; loader?: L; action?: A; - component?: Component, NoInfer>; + Component?: Component, NoInfer>; }) => route; From 6427d97451132e931f67776d156db629f825785e Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Tue, 4 Jun 2024 15:01:22 -0400 Subject: [PATCH 4/8] wip --- packages/remix-dev/vite/define-route.ts | 161 +++++++++++++++++------- packages/remix-dev/vite/plugin.ts | 106 +++++++++++----- 2 files changed, 191 insertions(+), 76 deletions(-) diff --git a/packages/remix-dev/vite/define-route.ts b/packages/remix-dev/vite/define-route.ts index 2274fdc7cc..e265d17362 100644 --- a/packages/remix-dev/vite/define-route.ts +++ b/packages/remix-dev/vite/define-route.ts @@ -3,14 +3,109 @@ import { parse, traverse, generate, t } from "./babel"; import type { GeneratorResult } from "@babel/generator"; import { deadCodeElimination, findReferencedIdentifiers } from "./dce"; -// given a route module filepath -// determine if its using `defineRoute$` -// if not, just do the old stuff that looks at exports - const MACRO = "defineRoute$"; const MACRO_PKG = "react-router"; const SERVER_ONLY_PROPERTIES = ["loader", "action"]; +type Field = NodePath; +type Fields = { + params?: NodePath; + loader?: Field; + clientLoader?: Field; + action?: Field; + clientAction?: Field; + Component?: Field; + ErrorBoundary?: Field; +}; + +export type HasFields = { + hasLoader: boolean; + hasClientLoader: boolean; + hasAction: boolean; + hasClientAction: boolean; + hasErrorBoundary: boolean; +}; + +function analyzeRoute(path: NodePath): Fields { + if (path.node.arguments.length !== 1) { + throw path.buildCodeFrameError(`macro must take exactly one argument`); + } + let arg = path.node.arguments[0]; + let argPath = path.get("arguments.0") as NodePath; + if (!t.isObjectExpression(arg)) { + throw argPath.buildCodeFrameError( + "macro argument must be a literal object" + ); + } + + let fields: Fields = {}; + for (let [i, property] of arg.properties.entries()) { + if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { + let propertyPath = argPath.get(`properties.${i}`) as NodePath; + throw propertyPath.buildCodeFrameError( + "macro argument must only have statically analyzable properties" + ); + } + let propertyPath = argPath.get(`properties.${i}`) as NodePath< + t.ObjectProperty | t.ObjectMethod + >; + if (property.computed || !t.isIdentifier(property.key)) { + throw propertyPath.buildCodeFrameError( + "macro argument must only have statically analyzable fields" + ); + } + + let key = property.key.name; + if (key === "params") { + let paramsPath = propertyPath as NodePath; + checkRouteParams(paramsPath); + fields["params"] = paramsPath; + continue; + } + + if ( + key === "loader" || + key === "clientLoader" || + key === "action" || + key === "clientAction" || + key === "Component" || + key === "ErrorBoundary" + ) { + fields[key] = propertyPath; + } + } + return fields; +} + +export function getDefineRouteHasFields(source: string): HasFields | null { + let ast = parse(source, { sourceType: "module" }); + let foundMacro = false; + let metadata: HasFields = { + hasLoader: false, + hasClientAction: false, + hasAction: false, + hasClientLoader: false, + hasErrorBoundary: false, + }; + traverse(ast, { + CallExpression(path) { + if (!isMacro(path)) return; + + foundMacro = true; + let fields = analyzeRoute(path); + metadata = { + hasLoader: fields.loader !== undefined, + hasClientLoader: fields.clientLoader !== undefined, + hasAction: fields.action !== undefined, + hasClientAction: fields.clientAction !== undefined, + hasErrorBoundary: fields.ErrorBoundary !== undefined, + }; + }, + }); + if (!foundMacro) return null; + return metadata; +} + export function transform( source: string, id: string, @@ -31,48 +126,18 @@ export function transform( ); }, CallExpression(path) { - if (!t.isIdentifier(path.node.callee)) return false; - let macro = path.node.callee.name; - let binding = path.scope.getBinding(macro); - - if (!binding) return false; - if (!isMacroBinding(binding)) return false; - if (path.node.arguments.length !== 1) { - throw path.buildCodeFrameError( - `'${macro}' macro must take exactly one argument` - ); - } - let arg = path.node.arguments[0]; - let argPath = path.get("arguments.0") as NodePath; - if (!t.isObjectExpression(arg)) { - throw argPath.buildCodeFrameError( - `'${macro}' macro argument must be a literal object` - ); - } + if (!isMacro(path)) return; - let markedForRemoval: NodePath[] = []; - for (let [i, property] of arg.properties.entries()) { - let propertyPath = argPath.get(`properties.${i}`) as NodePath; - if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { - throw propertyPath.buildCodeFrameError( - `'${macro}' macro argument must only have statically analyzable properties` - ); - } - if (property.computed || !t.isIdentifier(property.key)) { - throw propertyPath.buildCodeFrameError( - `'${macro}' macro argument must only have statically analyzable fields` - ); - } - - if (property.key.name === "params") { - checkRouteParams(propertyPath as NodePath); - } - - if (options.ssr && SERVER_ONLY_PROPERTIES.includes(property.key.name)) { - markedForRemoval.push(propertyPath); + let fields = analyzeRoute(path); + if (options.ssr) { + let markedForRemoval: NodePath[] = []; + for (let [key, fieldPath] of Object.entries(fields)) { + if (SERVER_ONLY_PROPERTIES.includes(key)) { + markedForRemoval.push(fieldPath); + } } + markedForRemoval.forEach((path) => path.remove()); } - markedForRemoval.forEach((path) => path.remove()); }, }); deadCodeElimination(ast, refs); @@ -96,6 +161,16 @@ function checkRouteParams(path: NodePath) { } } +function isMacro(path: NodePath): boolean { + if (!t.isIdentifier(path.node.callee)) return false; + let name = path.node.callee.name; + let binding = path.scope.getBinding(name); + + if (!binding) return false; + if (!isMacroBinding(binding)) return false; + return true; +} + function isMacroBinding(binding: Binding): boolean { // import source if (!t.isImportDeclaration(binding?.path.parent)) return false; diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 9e876daae0..2519f722b3 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -38,6 +38,8 @@ import { resolveEntryFiles, resolvePublicPath, } from "../config"; +import type { HasFields } from "./define-route"; +import { getDefineRouteHasFields } from "./define-route"; export async function resolveViteConfig({ configFile, @@ -283,18 +285,21 @@ const writeFileSafe = async (file: string, contents: string): Promise => { await fse.writeFile(file, contents); }; +// TODO: change this to return > => { +): Promise> => { let entries = await Promise.all( Object.entries(ctx.reactRouterConfig.routes).map(async ([key, route]) => { - let sourceExports = await getRouteModuleExports( + // do the old thing when `defineRoute$` isn't present + // otherwise use the new AST-based `has*` fields detector + let hasFields = await getRouteModuleHasFields({ viteChildCompiler, ctx, - route.file - ); - return [key, sourceExports] as const; + routeFile: route.file, + }); + return [key, hasFields] as const; }) ); return Object.fromEntries(entries); @@ -588,7 +593,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { ctx.reactRouterConfig.appDirectory, route.file ); - let sourceExports = routeManifestExports[key]; + let hasFields = routeManifestExports[key]; let isRootRoute = route.parentId === undefined; let routeManifestEntry = { @@ -597,11 +602,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { path: route.path, index: route.index, caseSensitive: route.caseSensitive, - hasAction: sourceExports.includes("action"), - hasLoader: sourceExports.includes("loader"), - hasClientAction: sourceExports.includes("clientAction"), - hasClientLoader: sourceExports.includes("clientLoader"), - hasErrorBoundary: sourceExports.includes("ErrorBoundary"), + ...hasFields, ...getReactRouterManifestBuildAssets( ctx, viteManifest, @@ -665,7 +666,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { ); for (let [key, route] of Object.entries(ctx.reactRouterConfig.routes)) { - let sourceExports = routeManifestExports[key]; + let hasFields = routeManifestExports[key]; routes[key] = { id: route.id, parentId: route.parentId, @@ -679,11 +680,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { resolveRelativeRouteFilePath(route, ctx.reactRouterConfig) )}` ), - hasAction: sourceExports.includes("action"), - hasLoader: sourceExports.includes("loader"), - hasClientAction: sourceExports.includes("clientAction"), - hasClientLoader: sourceExports.includes("clientLoader"), - hasErrorBoundary: sourceExports.includes("ErrorBoundary"), + ...hasFields, imports: [], }; } @@ -987,16 +984,21 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { if (id.endsWith(BUILD_CLIENT_ROUTE_QUERY_STRING)) { let routeModuleId = id.replace(BUILD_CLIENT_ROUTE_QUERY_STRING, ""); - let sourceExports = await getRouteModuleExports( + // TODO: make this work with new hasFields codepaths for defineRoute$ compat + let hasFields = await getRouteModuleHasFields({ viteChildCompiler, ctx, - routeModuleId - ); + routeFile: routeModuleId, + }); + + let clientExports = CLIENT_ROUTE_EXPORTS.filter( + (xport) => xport // TODO: make a fields util and a hasFields util, don't precompute hasFields, precompute _fields_ + ).join(", "); let routeFileName = path.basename(routeModuleId); - let clientExports = sourceExports - .filter((exportName) => CLIENT_ROUTE_EXPORTS.includes(exportName)) - .join(", "); + // let clientExports = sourceExports + // .filter((exportName) => CLIENT_ROUTE_EXPORTS.includes(exportName)) + // .join(", "); return `export { ${clientExports} } from "./${routeFileName}";`; } @@ -1612,12 +1614,12 @@ async function getRouteMetadata( route: ConfigRoute, readRouteFile?: () => string | Promise ) { - let sourceExports = await getRouteModuleExports( - viteChildCompiler, + let hasFields = await getRouteModuleHasFields({ ctx, - route.file, - readRouteFile - ); + viteChildCompiler, + routeFile: route.file, + readRouteFile, + }); let info = { id: route.id, @@ -1640,11 +1642,7 @@ async function getRouteMetadata( resolveRelativeRouteFilePath(route, ctx.reactRouterConfig) )}?import` ), // Ensure the Vite dev server responds with a JS module - hasAction: sourceExports.includes("action"), - hasClientAction: sourceExports.includes("clientAction"), - hasLoader: sourceExports.includes("loader"), - hasClientLoader: sourceExports.includes("clientLoader"), - hasErrorBoundary: sourceExports.includes("ErrorBoundary"), + ...hasFields, imports: [], }; return info; @@ -1877,3 +1875,45 @@ function createPrerenderRoutes( }; }); } + +async function getRouteModuleHasFieldsFromExports(args: { + viteChildCompiler: Vite.ViteDevServer | null; + ctx: ReactRouterPluginContext; + routeFile: string; + readRouteFile?: () => string | Promise; +}): Promise { + let xports = await getRouteModuleExports( + args.viteChildCompiler, + args.ctx, + args.routeFile, + args.readRouteFile + ); + + return { + hasLoader: xports.includes("loader"), + hasClientLoader: xports.includes("clientLoader"), + hasAction: xports.includes("action"), + hasClientAction: xports.includes("clientAction"), + hasErrorBoundary: xports.includes("ErrorBoundary"), + }; +} + +async function getRouteModuleHasFields(args: { + routeFile: string; + readRouteFile?: () => string | Promise; + + // optional for back compat + viteChildCompiler: Vite.ViteDevServer | null; + ctx: ReactRouterPluginContext; +}): Promise { + if (!args.routeFile.includes(`defineRoute$`)) { + return getRouteModuleHasFieldsFromExports(args); + } + + // TODO: probably also need to pass `readRouteFile` to `getDefineRouteHasFields` + let fields = getDefineRouteHasFields(args.routeFile); + if (!fields) { + return getRouteModuleHasFieldsFromExports(args); + } + return fields; +} From 89fb2f784acfda3a5085ae9578e837000f04ec51 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Tue, 4 Jun 2024 15:13:03 -0400 Subject: [PATCH 5/8] use babel-dead-code-elimination --- packages/remix-dev/package.json | 5 +- packages/remix-dev/vite/dce.ts | 285 ------------------------ packages/remix-dev/vite/define-route.ts | 5 +- pnpm-lock.yaml | 92 ++------ 4 files changed, 27 insertions(+), 360 deletions(-) delete mode 100644 packages/remix-dev/vite/dce.ts diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 907ab0bc9c..e6d1b77d74 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -33,6 +33,7 @@ "@react-router/node": "workspace:*", "@react-router/server-runtime": "workspace:*", "arg": "^5.0.1", + "babel-dead-code-elimination": "^1.0.0", "chalk": "^4.1.2", "es-module-lexer": "^1.3.1", "exit-hook": "2.2.1", @@ -65,9 +66,9 @@ "@types/prettier": "^2.7.3", "@types/set-cookie-parser": "^2.4.1", "dotenv": "^16.0.0", + "esbuild-register": "^3.3.2", "execa": "5.1.1", "express": "^4.17.1", - "esbuild-register": "^3.3.2", "fast-glob": "3.2.11", "strip-ansi": "^6.0.1", "tiny-invariant": "^1.2.0", @@ -75,8 +76,8 @@ "wrangler": "^3.28.2" }, "peerDependencies": { - "react-router": "workspace:^", "@react-router/serve": "workspace:^", + "react-router": "workspace:^", "typescript": "^5.1.0", "vite": "^5.1.0", "wrangler": "^3.28.2" diff --git a/packages/remix-dev/vite/dce.ts b/packages/remix-dev/vite/dce.ts deleted file mode 100644 index ec8fac47bd..0000000000 --- a/packages/remix-dev/vite/dce.ts +++ /dev/null @@ -1,285 +0,0 @@ -// Adapted from https://github.com/egoist/babel-plugin-eliminator/blob/d29859396b7708b7f7abbacdd951cbbc80902f00/src/index.ts - -import type { ParseResult } from "@babel/parser"; -import { traverse, t, type NodePath, type BabelTypes } from "./babel"; - -type IdentifierPath = NodePath; - -export function findReferencedIdentifiers( - ast: ParseResult -): Set { - const refs = new Set(); - - function markFunction( - path: NodePath< - | BabelTypes.FunctionDeclaration - | BabelTypes.FunctionExpression - | BabelTypes.ArrowFunctionExpression - > - ) { - const ident = getIdentifier(path); - if (ident?.node && isIdentifierReferenced(ident)) { - refs.add(ident); - } - } - - function markImport( - path: NodePath< - | BabelTypes.ImportSpecifier - | BabelTypes.ImportDefaultSpecifier - | BabelTypes.ImportNamespaceSpecifier - > - ) { - const local = path.get("local"); - if (isIdentifierReferenced(local)) { - refs.add(local); - } - } - - traverse(ast, { - VariableDeclarator(path) { - if (path.node.id.type === "Identifier") { - const local = path.get("id") as NodePath; - if (isIdentifierReferenced(local)) { - refs.add(local); - } - } else if (path.node.id.type === "ObjectPattern") { - const pattern = path.get("id") as NodePath; - - const properties = pattern.get("properties"); - properties.forEach((p) => { - const local = p.get( - p.node.type === "ObjectProperty" - ? "value" - : p.node.type === "RestElement" - ? "argument" - : (function () { - throw new Error("invariant"); - })() - ) as NodePath; - if (isIdentifierReferenced(local)) { - refs.add(local); - } - }); - } else if (path.node.id.type === "ArrayPattern") { - const pattern = path.get("id") as NodePath; - - const elements = pattern.get("elements"); - elements.forEach((e) => { - let local: NodePath; - if (e.node?.type === "Identifier") { - local = e as NodePath; - } else if (e.node?.type === "RestElement") { - local = e.get("argument") as NodePath; - } else { - return; - } - - if (isIdentifierReferenced(local)) { - refs.add(local); - } - }); - } - }, - - FunctionDeclaration: markFunction, - FunctionExpression: markFunction, - ArrowFunctionExpression: markFunction, - ImportSpecifier: markImport, - ImportDefaultSpecifier: markImport, - ImportNamespaceSpecifier: markImport, - }); - return refs; -} - -/** - * @param refs - If provided, only these identifiers will be considered for removal. - */ -export const deadCodeElimination = ( - ast: ParseResult, - refs?: Set -) => { - let referencesRemovedInThisPass: number; - - let shouldBeRemoved = (ident: IdentifierPath) => { - if (isIdentifierReferenced(ident)) return false; - if (!refs) return true; - return refs.has(ident); - }; - - let sweepFunction = ( - path: NodePath< - | BabelTypes.FunctionDeclaration - | BabelTypes.FunctionExpression - | BabelTypes.ArrowFunctionExpression - > - ) => { - let identifier = getIdentifier(path); - if (identifier?.node && shouldBeRemoved(identifier)) { - ++referencesRemovedInThisPass; - - if ( - t.isAssignmentExpression(path.parentPath.node) || - t.isVariableDeclarator(path.parentPath.node) - ) { - path.parentPath.remove(); - } else { - path.remove(); - } - } - }; - - let sweepImport = ( - path: NodePath< - | BabelTypes.ImportSpecifier - | BabelTypes.ImportDefaultSpecifier - | BabelTypes.ImportNamespaceSpecifier - > - ) => { - let local = path.get("local"); - if (shouldBeRemoved(local)) { - ++referencesRemovedInThisPass; - path.remove(); - if ( - (path.parent as BabelTypes.ImportDeclaration).specifiers.length === 0 - ) { - path.parentPath.remove(); - } - } - }; - - // Traverse again to remove unused references. This happens at least once, - // then repeats until no more references are removed. - do { - referencesRemovedInThisPass = 0; - - traverse(ast, { - Program(path) { - path.scope.crawl(); - }, - // eslint-disable-next-line no-loop-func - VariableDeclarator(path) { - if (path.node.id.type === "Identifier") { - let local = path.get("id") as NodePath; - if (shouldBeRemoved(local)) { - ++referencesRemovedInThisPass; - path.remove(); - } - } else if (path.node.id.type === "ObjectPattern") { - let pattern = path.get("id") as NodePath; - - let beforeCount = referencesRemovedInThisPass; - let properties = pattern.get("properties"); - properties.forEach((property) => { - let local = property.get( - property.node.type === "ObjectProperty" - ? "value" - : property.node.type === "RestElement" - ? "argument" - : (function () { - throw new Error("invariant"); - })() - ) as NodePath; - - if (shouldBeRemoved(local)) { - ++referencesRemovedInThisPass; - property.remove(); - } - }); - - if ( - beforeCount !== referencesRemovedInThisPass && - pattern.get("properties").length < 1 - ) { - path.remove(); - } - } else if (path.node.id.type === "ArrayPattern") { - let pattern = path.get("id") as NodePath; - - let beforeCount = referencesRemovedInThisPass; - let elements = pattern.get("elements"); - elements.forEach((e) => { - let local: NodePath; - if (e.node?.type === "Identifier") { - local = e as NodePath; - } else if (e.node?.type === "RestElement") { - local = e.get("argument") as NodePath; - } else { - return; - } - - if (shouldBeRemoved(local)) { - ++referencesRemovedInThisPass; - e.remove(); - } - }); - - if ( - beforeCount !== referencesRemovedInThisPass && - pattern.get("elements").length < 1 - ) { - path.remove(); - } - } - }, - FunctionDeclaration: sweepFunction, - FunctionExpression: sweepFunction, - ArrowFunctionExpression: sweepFunction, - ImportSpecifier: sweepImport, - ImportDefaultSpecifier: sweepImport, - ImportNamespaceSpecifier: sweepImport, - }); - } while (referencesRemovedInThisPass); -}; - -function getIdentifier( - path: NodePath< - | BabelTypes.FunctionDeclaration - | BabelTypes.FunctionExpression - | BabelTypes.ArrowFunctionExpression - > -): NodePath | null { - let parentPath = path.parentPath; - if (parentPath.type === "VariableDeclarator") { - let variablePath = parentPath as NodePath; - let name = variablePath.get("id"); - return name.node.type === "Identifier" - ? (name as NodePath) - : null; - } - - if (parentPath.type === "AssignmentExpression") { - let variablePath = parentPath as NodePath; - let name = variablePath.get("left"); - return name.node.type === "Identifier" - ? (name as NodePath) - : null; - } - - if (path.node.type === "ArrowFunctionExpression") { - return null; - } - - return path.node.id && path.node.id.type === "Identifier" - ? (path.get("id") as NodePath) - : null; -} - -function isIdentifierReferenced( - ident: NodePath -): boolean { - let binding = ident.scope.getBinding(ident.node.name); - if (binding?.referenced) { - // Functions can reference themselves, so we need to check if there's a - // binding outside the function scope or not. - if (binding.path.type === "FunctionDeclaration") { - return !binding.constantViolations - .concat(binding.referencePaths) - // Check that every reference is contained within the function: - .every((ref) => ref.findParent((parent) => parent === binding?.path)); - } - - return true; - } - return false; -} diff --git a/packages/remix-dev/vite/define-route.ts b/packages/remix-dev/vite/define-route.ts index e265d17362..c2454440df 100644 --- a/packages/remix-dev/vite/define-route.ts +++ b/packages/remix-dev/vite/define-route.ts @@ -1,7 +1,10 @@ import type { Binding, NodePath } from "@babel/traverse"; import { parse, traverse, generate, t } from "./babel"; import type { GeneratorResult } from "@babel/generator"; -import { deadCodeElimination, findReferencedIdentifiers } from "./dce"; +import { + deadCodeElimination, + findReferencedIdentifiers, +} from "babel-dead-code-elimination"; const MACRO = "defineRoute$"; const MACRO_PKG = "react-router"; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 769504b42d..f8cd56f9b1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -499,7 +499,7 @@ importers: version: 5.1.6 vite: specifier: ^5.1.0 - version: 5.1.3 + version: 5.1.3(@types/node@18.19.26) vite-env-only: specifier: ^2.0.0 version: 2.2.1 @@ -508,7 +508,7 @@ importers: version: 4.3.2(typescript@5.1.6)(vite@5.1.3) wrangler: specifier: ^3.24.0 - version: 3.39.0 + version: 3.39.0(@cloudflare/workers-types@4.20240403.0) packages/react-router: dependencies: @@ -574,6 +574,9 @@ importers: arg: specifier: ^5.0.1 version: 5.0.2 + babel-dead-code-elimination: + specifier: ^1.0.0 + version: 1.0.0 chalk: specifier: ^4.1.2 version: 4.1.2 @@ -6040,7 +6043,7 @@ packages: lodash: 4.17.21 mlly: 1.6.1 outdent: 0.8.0 - vite: 5.1.3 + vite: 5.1.3(@types/node@18.19.26) vite-node: 1.4.0 transitivePeerDependencies: - '@types/node' @@ -6064,7 +6067,7 @@ packages: outdent: 0.8.0 postcss: 8.4.38 postcss-load-config: 4.0.2(postcss@8.4.38) - vite: 5.1.3 + vite: 5.1.3(@types/node@18.19.26) transitivePeerDependencies: - '@types/node' - less @@ -6408,6 +6411,17 @@ packages: dequal: 2.0.3 dev: false + /babel-dead-code-elimination@1.0.0: + resolution: {integrity: sha512-6Z905/y0n1Hq15U0bdbUosrLPGD1s7G9Wj2GFBeLawO/6h4CCze6GWn3b1YAHFwXw1E7R0PieuGB3gkQOGP9TQ==} + dependencies: + '@babel/core': 7.24.3 + '@babel/parser': 7.24.1 + '@babel/traverse': 7.24.1 + '@babel/types': 7.24.0 + transitivePeerDependencies: + - supports-color + dev: false + /babel-jest@29.7.0(@babel/core@7.22.9): resolution: {integrity: sha512-BrvGY3xZSwEcCzKvKsCi2GgHqDqsYkOP4/by5xCgIwGXQxIEh+8ew3gmrE1y7XRR6LHZIj6yLYnUi/mm2KXKBg==} engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} @@ -13831,7 +13845,7 @@ packages: debug: 4.3.4 pathe: 1.1.2 picocolors: 1.0.0 - vite: 5.1.3 + vite: 5.1.3(@types/node@18.19.26) transitivePeerDependencies: - '@types/node' - less @@ -13853,7 +13867,7 @@ packages: debug: 4.3.4 globrex: 0.1.2 tsconfck: 3.0.3(typescript@5.1.6) - vite: 5.1.3 + vite: 5.1.3(@types/node@18.19.26) transitivePeerDependencies: - supports-color - typescript @@ -13874,40 +13888,6 @@ packages: - supports-color - typescript - /vite@5.1.3: - resolution: {integrity: sha512-UfmUD36DKkqhi/F75RrxvPpry+9+tTkrXfMNZD+SboZqBCMsxKtO52XeGzzuh7ioz+Eo/SYDBbdb0Z7vgcDJew==} - engines: {node: ^18.0.0 || >=20.0.0} - hasBin: true - peerDependencies: - '@types/node': ^18.0.0 || >=20.0.0 - less: '*' - lightningcss: ^1.21.0 - sass: '*' - stylus: '*' - sugarss: '*' - terser: ^5.4.0 - peerDependenciesMeta: - '@types/node': - optional: true - less: - optional: true - lightningcss: - optional: true - sass: - optional: true - stylus: - optional: true - sugarss: - optional: true - terser: - optional: true - dependencies: - esbuild: 0.19.12 - postcss: 8.4.38 - rollup: 4.13.1 - optionalDependencies: - fsevents: 2.3.3 - /vite@5.1.3(@types/node@18.19.26): resolution: {integrity: sha512-UfmUD36DKkqhi/F75RrxvPpry+9+tTkrXfMNZD+SboZqBCMsxKtO52XeGzzuh7ioz+Eo/SYDBbdb0Z7vgcDJew==} engines: {node: ^18.0.0 || >=20.0.0} @@ -14126,38 +14106,6 @@ packages: '@cloudflare/workerd-linux-arm64': 1.20240320.1 '@cloudflare/workerd-windows-64': 1.20240320.1 - /wrangler@3.39.0: - resolution: {integrity: sha512-bcSuZSsC2FCLrdtpFqAu1vV993wG+z3zASbZlPP1iq1VhJyuWeRS3GlObtCrbgRTJIva1T+ZOLCno8kaMSc8Pg==} - engines: {node: '>=16.17.0'} - hasBin: true - peerDependencies: - '@cloudflare/workers-types': ^4.20240320.1 - peerDependenciesMeta: - '@cloudflare/workers-types': - optional: true - dependencies: - '@cloudflare/kv-asset-handler': 0.3.1 - '@esbuild-plugins/node-globals-polyfill': 0.2.3(esbuild@0.17.19) - '@esbuild-plugins/node-modules-polyfill': 0.2.2(esbuild@0.17.19) - blake3-wasm: 2.1.5 - chokidar: 3.6.0 - esbuild: 0.17.19 - miniflare: 3.20240320.0 - nanoid: 3.3.7 - path-to-regexp: 6.2.1 - resolve: 1.22.8 - resolve.exports: 2.0.2 - selfsigned: 2.4.1 - source-map: 0.6.1 - xxhash-wasm: 1.0.2 - optionalDependencies: - fsevents: 2.3.3 - transitivePeerDependencies: - - bufferutil - - supports-color - - utf-8-validate - dev: true - /wrangler@3.39.0(@cloudflare/workers-types@4.20240403.0): resolution: {integrity: sha512-bcSuZSsC2FCLrdtpFqAu1vV993wG+z3zASbZlPP1iq1VhJyuWeRS3GlObtCrbgRTJIva1T+ZOLCno8kaMSc8Pg==} engines: {node: '>=16.17.0'} From 59ecf452f9492c11403d2d563347768c1099cb82 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 6 Jun 2024 08:57:47 -0400 Subject: [PATCH 6/8] wip --- .../react-router/lib/router/define-route.ts | 2 +- packages/remix-dev/package.json | 2 +- packages/remix-dev/vite/define-route.ts | 274 ++++++++++-------- packages/remix-dev/vite/plugin.ts | 189 +++++------- pnpm-lock.yaml | 8 +- 5 files changed, 230 insertions(+), 245 deletions(-) diff --git a/packages/react-router/lib/router/define-route.ts b/packages/react-router/lib/router/define-route.ts index 57e4f32632..f680806a1a 100644 --- a/packages/react-router/lib/router/define-route.ts +++ b/packages/react-router/lib/router/define-route.ts @@ -57,7 +57,7 @@ type Component

> = (args: { // loader -> data for component // loader -> serverLoader for clientLoader -> data for component // TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) -export const defineRoute$ = < +export const defineRoute = < const P extends string, L extends Loader

, A extends Action

diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index e6d1b77d74..ba372389af 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -33,7 +33,7 @@ "@react-router/node": "workspace:*", "@react-router/server-runtime": "workspace:*", "arg": "^5.0.1", - "babel-dead-code-elimination": "^1.0.0", + "babel-dead-code-elimination": "^1.0.1", "chalk": "^4.1.2", "es-module-lexer": "^1.3.1", "exit-hook": "2.2.1", diff --git a/packages/remix-dev/vite/define-route.ts b/packages/remix-dev/vite/define-route.ts index c2454440df..800828563a 100644 --- a/packages/remix-dev/vite/define-route.ts +++ b/packages/remix-dev/vite/define-route.ts @@ -1,112 +1,62 @@ +import * as babel from "@babel/core"; import type { Binding, NodePath } from "@babel/traverse"; -import { parse, traverse, generate, t } from "./babel"; import type { GeneratorResult } from "@babel/generator"; import { deadCodeElimination, findReferencedIdentifiers, } from "babel-dead-code-elimination"; -const MACRO = "defineRoute$"; -const MACRO_PKG = "react-router"; -const SERVER_ONLY_PROPERTIES = ["loader", "action"]; - -type Field = NodePath; -type Fields = { - params?: NodePath; - loader?: Field; - clientLoader?: Field; - action?: Field; - clientAction?: Field; - Component?: Field; - ErrorBoundary?: Field; -}; - -export type HasFields = { - hasLoader: boolean; - hasClientLoader: boolean; - hasAction: boolean; - hasClientAction: boolean; - hasErrorBoundary: boolean; -}; - -function analyzeRoute(path: NodePath): Fields { - if (path.node.arguments.length !== 1) { - throw path.buildCodeFrameError(`macro must take exactly one argument`); - } - let arg = path.node.arguments[0]; - let argPath = path.get("arguments.0") as NodePath; - if (!t.isObjectExpression(arg)) { - throw argPath.buildCodeFrameError( - "macro argument must be a literal object" - ); - } +import { generate, parse, t, traverse } from "./babel"; - let fields: Fields = {}; - for (let [i, property] of arg.properties.entries()) { - if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { - let propertyPath = argPath.get(`properties.${i}`) as NodePath; - throw propertyPath.buildCodeFrameError( - "macro argument must only have statically analyzable properties" - ); - } - let propertyPath = argPath.get(`properties.${i}`) as NodePath< - t.ObjectProperty | t.ObjectMethod - >; - if (property.computed || !t.isIdentifier(property.key)) { - throw propertyPath.buildCodeFrameError( - "macro argument must only have statically analyzable fields" - ); - } +function parseRoute(source: string) { + let ast = parse(source, { + sourceType: "module", + plugins: ["jsx", ["typescript", {}]], + }); - let key = property.key.name; - if (key === "params") { - let paramsPath = propertyPath as NodePath; - checkRouteParams(paramsPath); - fields["params"] = paramsPath; - continue; - } + // Workaround for `path.buildCodeFrameError` + // See: + // - https://github.com/babel/babel/issues/11889 + // - https://github.com/babel/babel/issues/11350#issuecomment-606169054 + // @ts-expect-error `@types/babel__core` is missing types for `File` + new babel.File({ filename: undefined }, { code: source, ast }); - if ( - key === "loader" || - key === "clientLoader" || - key === "action" || - key === "clientAction" || - key === "Component" || - key === "ErrorBoundary" - ) { - fields[key] = propertyPath; - } - } - return fields; + return ast; } -export function getDefineRouteHasFields(source: string): HasFields | null { - let ast = parse(source, { sourceType: "module" }); - let foundMacro = false; - let metadata: HasFields = { - hasLoader: false, - hasClientAction: false, - hasAction: false, - hasClientLoader: false, - hasErrorBoundary: false, - }; +let fields = [ + "loader", + "clientLoader", + "action", + "clientAction", + "Component", + "ErrorBoundary", +] as const; + +type Field = (typeof fields)[number]; +type FieldPath = NodePath; + +function isField(field: string): field is Field { + return (fields as readonly string[]).includes(field); +} + +type Analysis = Record; + +export function parseRouteFields(source: string): string[] { + let ast = parseRoute(source); + + let fieldNames: Field[] = []; traverse(ast, { - CallExpression(path) { - if (!isMacro(path)) return; - - foundMacro = true; - let fields = analyzeRoute(path); - metadata = { - hasLoader: fields.loader !== undefined, - hasClientLoader: fields.clientLoader !== undefined, - hasAction: fields.action !== undefined, - hasClientAction: fields.clientAction !== undefined, - hasErrorBoundary: fields.ErrorBoundary !== undefined, - }; + ExportDefaultDeclaration(path) { + let fields = analyzeRouteExport(path); + if (fields instanceof Error) throw fields; + for (let [key, fieldPath] of Object.entries(fields)) { + if (!fieldPath) continue; + fieldNames.push(key as Field); + } }, }); - if (!foundMacro) return null; - return metadata; + return fieldNames; } export function transform( @@ -114,39 +64,102 @@ export function transform( id: string, options: { ssr: boolean } ): GeneratorResult { - let ast = parse(source, { sourceType: "module" }); - let refs = findReferencedIdentifiers(ast); + let ast = parseRoute(source); + let refs = findReferencedIdentifiers(ast); traverse(ast, { - Identifier(path) { - if (t.isImportSpecifier(path.parent)) return; - let binding = path.scope.getBinding(path.node.name); - if (!binding) return; - if (!isMacroBinding(binding)) return; - if (t.isCallExpression(path.parent)) return; - throw path.buildCodeFrameError( - `'${path.node.name}' macro cannot be manipulated at runtime as it must be statically analyzable` - ); - }, - CallExpression(path) { - if (!isMacro(path)) return; - - let fields = analyzeRoute(path); - if (options.ssr) { - let markedForRemoval: NodePath[] = []; - for (let [key, fieldPath] of Object.entries(fields)) { - if (SERVER_ONLY_PROPERTIES.includes(key)) { - markedForRemoval.push(fieldPath); - } + ExportDefaultDeclaration(path) { + let fields = analyzeRouteExport(path); + if (fields instanceof Error) throw fields; + if (options.ssr) return; + + let markedForRemoval: NodePath[] = []; + for (let [key, fieldPath] of Object.entries(fields)) { + if (["loader", "action"].includes(key)) { + if (!fieldPath) continue; + markedForRemoval.push(fieldPath); } - markedForRemoval.forEach((path) => path.remove()); } + markedForRemoval.forEach((path) => path.remove()); }, }); deadCodeElimination(ast, refs); return generate(ast, { sourceMaps: true, sourceFileName: id }, source); } +function analyzeRouteExport( + path: NodePath +): Analysis | Error { + let route = path.node.declaration; + + // export default {...} + if (t.isObjectExpression(route)) { + let routePath = path.get("declaration") as NodePath; + return analyzeRoute(routePath); + } + + // export default defineRoute({...}) + if (t.isCallExpression(route)) { + let routePath = path.get("declaration") as NodePath; + if (!isDefineRoute(routePath)) { + return routePath.buildCodeFrameError("TODO"); + } + + if (routePath.node.arguments.length !== 1) { + return path.buildCodeFrameError( + `defineRoute must take exactly one argument` + ); + } + let arg = routePath.node.arguments[0]; + let argPath = path.get("arguments.0") as NodePath; + if (!t.isObjectExpression(arg)) { + return argPath.buildCodeFrameError( + "defineRoute argument must be a literal object" + ); + } + return analyzeRoute(argPath); + } + + return path.get("declaration").buildCodeFrameError("TODO"); +} + +function analyzeRoute(path: NodePath): Analysis { + let analysis: Analysis = { + loader: null, + clientLoader: null, + action: null, + clientAction: null, + Component: null, + ErrorBoundary: null, + }; + + for (let [i, property] of path.node.properties.entries()) { + if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { + let propertyPath = path.get(`properties.${i}`) as NodePath; + throw propertyPath.buildCodeFrameError("todo"); + } + + let propertyPath = path.get(`properties.${i}`) as NodePath< + t.ObjectProperty | t.ObjectMethod + >; + if (property.computed || !t.isIdentifier(property.key)) { + throw propertyPath.buildCodeFrameError("todo"); + } + + let key = property.key.name; + if (key === "params") { + let paramsPath = propertyPath as NodePath; + checkRouteParams(paramsPath); + continue; + } + if (isField(key)) { + analysis[key] = propertyPath; + continue; + } + } + return analysis; +} + function checkRouteParams(path: NodePath) { if (t.isObjectMethod(path.node)) { throw path.buildCodeFrameError(`params must be statically analyzable`); @@ -164,25 +177,34 @@ function checkRouteParams(path: NodePath) { } } -function isMacro(path: NodePath): boolean { +function isDefineRoute(path: NodePath): boolean { if (!t.isIdentifier(path.node.callee)) return false; - let name = path.node.callee.name; - let binding = path.scope.getBinding(name); - + let binding = path.scope.getBinding(path.node.callee.name); if (!binding) return false; - if (!isMacroBinding(binding)) return false; - return true; + return isCanonicallyImportedAs(binding, { + imported: "defineRoute", + source: "react-router", + }); } -function isMacroBinding(binding: Binding): boolean { +function isCanonicallyImportedAs( + binding: Binding, + { + source: sourceName, + imported: importedName, + }: { + source: string; + imported: string; + } +): boolean { // import source if (!t.isImportDeclaration(binding?.path.parent)) return false; - if (binding.path.parent.source.value !== MACRO_PKG) return false; + if (binding.path.parent.source.value !== sourceName) return false; // import specifier if (!t.isImportSpecifier(binding?.path.node)) return false; let { imported } = binding.path.node; if (!t.isIdentifier(imported)) return false; - if (imported.name !== MACRO) return false; + if (imported.name !== importedName) return false; return true; } diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 2519f722b3..d346559aa7 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -38,8 +38,7 @@ import { resolveEntryFiles, resolvePublicPath, } from "../config"; -import type { HasFields } from "./define-route"; -import { getDefineRouteHasFields } from "./define-route"; +import { parseRouteFields } from "./define-route"; export async function resolveViteConfig({ configFile, @@ -285,67 +284,21 @@ const writeFileSafe = async (file: string, contents: string): Promise => { await fse.writeFile(file, contents); }; -// TODO: change this to return > => { let entries = await Promise.all( Object.entries(ctx.reactRouterConfig.routes).map(async ([key, route]) => { - // do the old thing when `defineRoute$` isn't present - // otherwise use the new AST-based `has*` fields detector - let hasFields = await getRouteModuleHasFields({ - viteChildCompiler, - ctx, - routeFile: route.file, - }); + let source = await getRouteSource(ctx, route.file, viteChildCompiler); + let fields = parseRouteFields(source); + let hasFields = toHasFields(fields); return [key, hasFields] as const; }) ); return Object.fromEntries(entries); }; -const getRouteModuleExports = async ( - viteChildCompiler: Vite.ViteDevServer | null, - ctx: ReactRouterPluginContext, - routeFile: string, - readRouteFile?: () => string | Promise -): Promise => { - if (!viteChildCompiler) { - throw new Error("Vite child compiler not found"); - } - - // We transform the route module code with the Vite child compiler so that we - // can parse the exports from non-JS files like MDX. This ensures that we can - // understand the exports from anything that Vite can compile to JS, not just - // the route file formats that the Remix compiler historically supported. - - let ssr = true; - let { pluginContainer, moduleGraph } = viteChildCompiler; - - let routePath = path.resolve(ctx.reactRouterConfig.appDirectory, routeFile); - let url = resolveFileUrl(ctx, routePath); - - let resolveId = async () => { - let result = await pluginContainer.resolveId(url, undefined, { ssr }); - if (!result) throw new Error(`Could not resolve module ID for ${url}`); - return result.id; - }; - - let [id, code] = await Promise.all([ - resolveId(), - readRouteFile?.() ?? fse.readFile(routePath, "utf-8"), - // pluginContainer.transform(...) fails if we don't do this first: - moduleGraph.ensureEntryFromUrl(url, ssr), - ]); - - let transformed = await pluginContainer.transform(code, id, { ssr }); - let [, exports] = esModuleLexer(transformed.code); - let exportNames = exports.map((e) => e.n); - - return exportNames; -}; - const getServerBundleBuildConfig = ( viteUserConfig: Vite.UserConfig ): ServerBundleBuildConfig | null => { @@ -489,7 +442,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { : // Otherwise, all routes are imported as usual ctx.reactRouterConfig.routes; - return ` + let content = ` import * as entryServer from ${JSON.stringify( resolveFileUrl(ctx, ctx.entryServerFilePath) )}; @@ -533,6 +486,8 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { }) .join(",\n ")} };`; + + return content; }; let loadViteManifest = async (directory: string) => { @@ -584,8 +539,8 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { let serverRoutes: ReactRouterManifest["routes"] = {}; let routeManifestExports = await getRouteManifestModuleExports( - viteChildCompiler, - ctx + ctx, + viteChildCompiler ); for (let [key, route] of Object.entries(ctx.reactRouterConfig.routes)) { @@ -661,8 +616,8 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { let routes: ReactRouterManifest["routes"] = {}; let routeManifestExports = await getRouteManifestModuleExports( - viteChildCompiler, - ctx + ctx, + viteChildCompiler ); for (let [key, route] of Object.entries(ctx.reactRouterConfig.routes)) { @@ -944,7 +899,6 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { ); } } - viteChildCompiler = await vite.createServer({ ...viteUserConfig, mode: viteConfig.mode, @@ -984,21 +938,18 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { if (id.endsWith(BUILD_CLIENT_ROUTE_QUERY_STRING)) { let routeModuleId = id.replace(BUILD_CLIENT_ROUTE_QUERY_STRING, ""); - // TODO: make this work with new hasFields codepaths for defineRoute$ compat - let hasFields = await getRouteModuleHasFields({ - viteChildCompiler, + let source = await getRouteSource( ctx, - routeFile: routeModuleId, - }); + routeModuleId, + viteChildCompiler + ); + let fields = parseRouteFields(source); - let clientExports = CLIENT_ROUTE_EXPORTS.filter( - (xport) => xport // TODO: make a fields util and a hasFields util, don't precompute hasFields, precompute _fields_ - ).join(", "); + let clientExports = fields + .filter((field) => CLIENT_ROUTE_EXPORTS.includes(field)) + .join(", "); let routeFileName = path.basename(routeModuleId); - // let clientExports = sourceExports - // .filter((exportName) => CLIENT_ROUTE_EXPORTS.includes(exportName)) - // .join(", "); return `export { ${clientExports} } from "./${routeFileName}";`; } @@ -1481,8 +1432,8 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { let oldRouteMetadata = serverManifest.routes[route.id]; let newRouteMetadata = await getRouteMetadata( ctx, - viteChildCompiler, route, + viteChildCompiler, read ); @@ -1610,16 +1561,18 @@ function getRoute( async function getRouteMetadata( ctx: ReactRouterPluginContext, - viteChildCompiler: Vite.ViteDevServer | null, route: ConfigRoute, + viteChildCompiler: Vite.ViteDevServer | null, readRouteFile?: () => string | Promise ) { - let hasFields = await getRouteModuleHasFields({ + let source = await getRouteSource( ctx, + route.file, viteChildCompiler, - routeFile: route.file, - readRouteFile, - }); + readRouteFile + ); + let fields = parseRouteFields(source); + let hasFields = toHasFields(fields); let info = { id: route.id, @@ -1876,44 +1829,54 @@ function createPrerenderRoutes( }); } -async function getRouteModuleHasFieldsFromExports(args: { - viteChildCompiler: Vite.ViteDevServer | null; - ctx: ReactRouterPluginContext; - routeFile: string; - readRouteFile?: () => string | Promise; -}): Promise { - let xports = await getRouteModuleExports( - args.viteChildCompiler, - args.ctx, - args.routeFile, - args.readRouteFile - ); +async function getRouteSource( + ctx: ReactRouterPluginContext, + routeFile: string, + viteChildCompiler: Vite.ViteDevServer | null, + readRouteFile?: () => string | Promise +): Promise { + // TODO: if routeFile is not js/jsx/ts/tsx, use child compiler + // + // if (!viteChildCompiler) { + // throw new Error("Vite child compiler not found"); + // } + // let ssr = true; + // let { pluginContainer, moduleGraph } = viteChildCompiler; - return { - hasLoader: xports.includes("loader"), - hasClientLoader: xports.includes("clientLoader"), - hasAction: xports.includes("action"), - hasClientAction: xports.includes("clientAction"), - hasErrorBoundary: xports.includes("ErrorBoundary"), - }; + let routePath = path.resolve(ctx.reactRouterConfig.appDirectory, routeFile); + // let url = resolveFileUrl(ctx, routePath); + // let resolveId = async () => { + // let result = await pluginContainer.resolveId(url, undefined, { ssr }); + // if (!result) throw new Error(`Could not resolve module ID for ${url}`); + // return result.id; + // }; + + // let [id, code] = await Promise.all([ + // resolveId(), + // readRouteFile?.() ?? fse.readFile(routePath, "utf-8"), + // // pluginContainer.transform(...) fails if we don't do this first: + // moduleGraph.ensureEntryFromUrl(url, ssr), + // ]); + + // let transformed = await pluginContainer.transform(code, id, { ssr }); + + return readRouteFile?.() ?? fse.readFile(routePath, "utf-8"); + // return transformed.code; } -async function getRouteModuleHasFields(args: { - routeFile: string; - readRouteFile?: () => string | Promise; - - // optional for back compat - viteChildCompiler: Vite.ViteDevServer | null; - ctx: ReactRouterPluginContext; -}): Promise { - if (!args.routeFile.includes(`defineRoute$`)) { - return getRouteModuleHasFieldsFromExports(args); - } - - // TODO: probably also need to pass `readRouteFile` to `getDefineRouteHasFields` - let fields = getDefineRouteHasFields(args.routeFile); - if (!fields) { - return getRouteModuleHasFieldsFromExports(args); - } - return fields; +type HasFields = { + hasLoader: boolean; + hasClientLoader: boolean; + hasAction: boolean; + hasClientAction: boolean; + hasErrorBoundary: boolean; +}; +function toHasFields(fields: string[]) { + return { + hasLoader: fields.includes("loader"), + hasClientLoader: fields.includes("clientLoader"), + hasAction: fields.includes("action"), + hasClientAction: fields.includes("clientAction"), + hasErrorBoundary: fields.includes("ErrorBoundary"), + }; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f8cd56f9b1..625b73c3b3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -575,8 +575,8 @@ importers: specifier: ^5.0.1 version: 5.0.2 babel-dead-code-elimination: - specifier: ^1.0.0 - version: 1.0.0 + specifier: ^1.0.1 + version: 1.0.1 chalk: specifier: ^4.1.2 version: 4.1.2 @@ -6411,8 +6411,8 @@ packages: dequal: 2.0.3 dev: false - /babel-dead-code-elimination@1.0.0: - resolution: {integrity: sha512-6Z905/y0n1Hq15U0bdbUosrLPGD1s7G9Wj2GFBeLawO/6h4CCze6GWn3b1YAHFwXw1E7R0PieuGB3gkQOGP9TQ==} + /babel-dead-code-elimination@1.0.1: + resolution: {integrity: sha512-QD6IAGZU/Qd7qJJPptnPVGRl9SnK9IdowcIjOcxVKbfB70chvCXRaV2BOgxpVckQud3CppYoI6QA+/cfdBGAMA==} dependencies: '@babel/core': 7.24.3 '@babel/parser': 7.24.1 From 1a3dcdc0669643b816f771c1287c19b5eba7067a Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Thu, 6 Jun 2024 14:32:29 -0400 Subject: [PATCH 7/8] working in playground --- packages/react-router/index.ts | 1 + .../react-router/lib/dom/ssr/components.tsx | 5 +- .../react-router/lib/dom/ssr/routeModules.ts | 5 +- packages/react-router/lib/dom/ssr/routes.tsx | 30 ++++++++++-- .../react-router/lib/router/define-route.ts | 1 + packages/remix-dev/vite/define-route.ts | 47 ++++++++++++++----- packages/remix-dev/vite/plugin.ts | 30 ++---------- 7 files changed, 78 insertions(+), 41 deletions(-) diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 060af1c3fd..07baeba155 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -405,3 +405,4 @@ export { decodeViaTurboStream as UNSAFE_decodeViaTurboStream, SingleFetchRedirectSymbol as UNSAFE_SingleFetchRedirectSymbol, } from "./lib/dom/ssr/single-fetch"; +export { defineRoute } from "./lib/router/define-route"; diff --git a/packages/react-router/lib/dom/ssr/components.tsx b/packages/react-router/lib/dom/ssr/components.tsx index 48b2563e0e..0c82369627 100644 --- a/packages/react-router/lib/dom/ssr/components.tsx +++ b/packages/react-router/lib/dom/ssr/components.tsx @@ -642,7 +642,10 @@ ${matches .join("\n")} window.__remixRouteModules = {${matches .map( - (match, index) => `${JSON.stringify(match.route.id)}:route${index}` + (match, index) => + `${JSON.stringify( + match.route.id + )}: { ...route${index}.default, default: route${index}.default.Component, Component: undefined}` ) .join(",")}}; diff --git a/packages/react-router/lib/dom/ssr/routeModules.ts b/packages/react-router/lib/dom/ssr/routeModules.ts index e34bb20629..3e97bf748b 100644 --- a/packages/react-router/lib/dom/ssr/routeModules.ts +++ b/packages/react-router/lib/dom/ssr/routeModules.ts @@ -200,7 +200,10 @@ export async function loadRouteModule( } try { - let routeModule = await import(/* webpackIgnore: true */ route.module); + let { Component, ...routeModule } = ( + await import(/* webpackIgnore: true */ route.module) + ).default; + routeModule.default = Component; routeModulesCache[route.id] = routeModule; return routeModule; } catch (error: unknown) { diff --git a/packages/react-router/lib/dom/ssr/routes.tsx b/packages/react-router/lib/dom/ssr/routes.tsx index c80b75b550..f513b7b409 100644 --- a/packages/react-router/lib/dom/ssr/routes.tsx +++ b/packages/react-router/lib/dom/ssr/routes.tsx @@ -14,7 +14,12 @@ import { prefetchStyleLinks } from "./links"; import { RemixRootDefaultErrorBoundary } from "./errorBoundaries"; import { RemixRootDefaultHydrateFallback } from "./fallback"; import invariant from "./invariant"; -import { useRouteError } from "../../hooks"; +import { + useActionData, + useLoaderData, + useParams, + useRouteError, +} from "../../hooks"; import type { DataRouteObject } from "../../context"; export interface RouteManifest { @@ -64,7 +69,22 @@ function getRouteComponents( routeModule: RouteModule, isSpaMode: boolean ) { - let Component = getRouteModuleComponent(routeModule); + let ComponentWithoutData = getRouteModuleComponent(routeModule); + let Component = ComponentWithoutData + ? () => { + let params = useParams(); + let data = useLoaderData(); + let actionData = useActionData(); + return ( + + ); + } + : undefined; // HydrateFallback can only exist on the root route in SPA Mode let HydrateFallback = routeModule.HydrateFallback && (!isSpaMode || route.id === "root") @@ -110,7 +130,11 @@ function getRouteComponents( }; } - return { Component, ErrorBoundary, HydrateFallback }; + return { + Component, + ErrorBoundary, + HydrateFallback, + }; } export function createServerRoutes( diff --git a/packages/react-router/lib/router/define-route.ts b/packages/react-router/lib/router/define-route.ts index f680806a1a..06b17b1449 100644 --- a/packages/react-router/lib/router/define-route.ts +++ b/packages/react-router/lib/router/define-route.ts @@ -57,6 +57,7 @@ type Component

> = (args: { // loader -> data for component // loader -> serverLoader for clientLoader -> data for component // TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) +// TODO: `handle`, HydrateFallback, shouldRevalidate, Layout, meta, links export const defineRoute = < const P extends string, L extends Loader

, diff --git a/packages/remix-dev/vite/define-route.ts b/packages/remix-dev/vite/define-route.ts index 800828563a..d35f5d7073 100644 --- a/packages/remix-dev/vite/define-route.ts +++ b/packages/remix-dev/vite/define-route.ts @@ -24,13 +24,19 @@ function parseRoute(source: string) { return ast; } +// TODO: account for layout, let fields = [ + "meta", + "links", "loader", "clientLoader", "action", "clientAction", "Component", "ErrorBoundary", + "HydrateFallback", + "handle", + "shouldRevalidate", ] as const; type Field = (typeof fields)[number]; @@ -42,7 +48,7 @@ function isField(field: string): field is Field { type Analysis = Record; -export function parseRouteFields(source: string): string[] { +export function parseRouteFields(source: string): Field[] { let ast = parseRoute(source); let fieldNames: Field[] = []; @@ -102,48 +108,63 @@ function analyzeRouteExport( if (t.isCallExpression(route)) { let routePath = path.get("declaration") as NodePath; if (!isDefineRoute(routePath)) { - return routePath.buildCodeFrameError("TODO"); + return routePath.buildCodeFrameError( + "Default export of a route module must either be a literal object or a call to `defineRoute`" + ); } if (routePath.node.arguments.length !== 1) { return path.buildCodeFrameError( - `defineRoute must take exactly one argument` + "`defineRoute` must take exactly one argument" ); } let arg = routePath.node.arguments[0]; - let argPath = path.get("arguments.0") as NodePath; + let argPath = routePath.get("arguments.0") as NodePath; if (!t.isObjectExpression(arg)) { return argPath.buildCodeFrameError( - "defineRoute argument must be a literal object" + "`defineRoute` argument must be a literal object" ); } return analyzeRoute(argPath); } - return path.get("declaration").buildCodeFrameError("TODO"); + return path + .get("declaration") + .buildCodeFrameError( + "Default export of a route module must be either a literal object or a call to `defineRoute`" + ); } function analyzeRoute(path: NodePath): Analysis { let analysis: Analysis = { + meta: null, + links: null, loader: null, clientLoader: null, action: null, clientAction: null, Component: null, ErrorBoundary: null, + HydrateFallback: null, + handle: null, + shouldRevalidate: null, }; for (let [i, property] of path.node.properties.entries()) { if (!t.isObjectProperty(property) && !t.isObjectMethod(property)) { let propertyPath = path.get(`properties.${i}`) as NodePath; - throw propertyPath.buildCodeFrameError("todo"); + throw propertyPath.buildCodeFrameError( + "Route properties cannot have dynamically computed keys" + ); } let propertyPath = path.get(`properties.${i}`) as NodePath< t.ObjectProperty | t.ObjectMethod >; if (property.computed || !t.isIdentifier(property.key)) { - throw propertyPath.buildCodeFrameError("todo"); + throw propertyPath.buildCodeFrameError( + "Route properties cannot have dynamically computed keys" + ); } let key = property.key.name; @@ -162,16 +183,20 @@ function analyzeRoute(path: NodePath): Analysis { function checkRouteParams(path: NodePath) { if (t.isObjectMethod(path.node)) { - throw path.buildCodeFrameError(`params must be statically analyzable`); + throw path.buildCodeFrameError( + "Route params must be a literal array of literal strings" + ); } if (!t.isArrayExpression(path.node.value)) { - throw path.buildCodeFrameError(`params must be statically analyzable`); + throw path.buildCodeFrameError( + "Route params must be a literal array of literal strings" + ); } for (let [i, element] of path.node.value.elements.entries()) { if (!t.isStringLiteral(element)) { let elementPath = path.get(`value.elements.${i}`) as NodePath; throw elementPath.buildCodeFrameError( - `params must be statically analyzable` + "Route params must be a literal array of literal strings" ); } } diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index d346559aa7..6e9526a1e2 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -481,7 +481,11 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = (_config) => { path: ${JSON.stringify(route.path)}, index: ${JSON.stringify(route.index)}, caseSensitive: ${JSON.stringify(route.caseSensitive)}, - module: route${index} + module: { + ...route${index}.default, + default: route${index}.default.Component, + Component: undefined, + } }`; }) .join(",\n ")} @@ -1836,32 +1840,8 @@ async function getRouteSource( readRouteFile?: () => string | Promise ): Promise { // TODO: if routeFile is not js/jsx/ts/tsx, use child compiler - // - // if (!viteChildCompiler) { - // throw new Error("Vite child compiler not found"); - // } - // let ssr = true; - // let { pluginContainer, moduleGraph } = viteChildCompiler; - let routePath = path.resolve(ctx.reactRouterConfig.appDirectory, routeFile); - // let url = resolveFileUrl(ctx, routePath); - // let resolveId = async () => { - // let result = await pluginContainer.resolveId(url, undefined, { ssr }); - // if (!result) throw new Error(`Could not resolve module ID for ${url}`); - // return result.id; - // }; - - // let [id, code] = await Promise.all([ - // resolveId(), - // readRouteFile?.() ?? fse.readFile(routePath, "utf-8"), - // // pluginContainer.transform(...) fails if we don't do this first: - // moduleGraph.ensureEntryFromUrl(url, ssr), - // ]); - - // let transformed = await pluginContainer.transform(code, id, { ssr }); - return readRouteFile?.() ?? fse.readFile(routePath, "utf-8"); - // return transformed.code; } type HasFields = { From eef81c9cc8555f4f7e0881cfbdb18ddfaf9caaa5 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Mon, 17 Jun 2024 16:05:48 -0400 Subject: [PATCH 8/8] better types + tests --- .../lib/router/define-route.test.ts | 276 +++++++++++++++ .../react-router/lib/router/define-route.ts | 314 ++++++++++++++++-- 2 files changed, 564 insertions(+), 26 deletions(-) create mode 100644 packages/react-router/lib/router/define-route.test.ts diff --git a/packages/react-router/lib/router/define-route.test.ts b/packages/react-router/lib/router/define-route.test.ts new file mode 100644 index 0000000000..6e3534141e --- /dev/null +++ b/packages/react-router/lib/router/define-route.test.ts @@ -0,0 +1,276 @@ +import { defineRoute } from "./define-route"; + +// TODO: make sure tsc fails when there are type errors in this file + +// prettier-ignore +type Equal = + (() => T extends X ? 1 : 2) extends + (() => T extends Y ? 1 : 2) ? true : false +function expectEqual(_: Equal) {} + +// Infer params +type Params = { + [key: string]: string | undefined; + id: string; + brand?: string; +}; +defineRoute({ + params: ["id", "brand?"], + links({ params }) { + expectEqual(true); + return []; + }, + HydrateFallback({ params }) { + expectEqual(true); + return null; + }, + serverLoader({ params }) { + expectEqual(true); + return null; + }, + clientLoader({ params }) { + expectEqual(true); + return null; + }, + serverAction({ params }) { + expectEqual(true); + return null; + }, + clientAction({ params }) { + expectEqual(true); + return null; + }, + meta({ params }) { + expectEqual(true); + return []; + }, + Component({ params }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ params }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: no loaders -> undefined +defineRoute({ + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: server -> server +defineRoute({ + serverLoader() { + return 1; + }, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: server + client -> server | client +defineRoute({ + serverLoader() { + return 1; + }, + async clientLoader({ serverLoader }) { + let serverData = await serverLoader(); + expectEqual(true); + return 2 as const; + }, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: server + client + hydrate -> server | client +defineRoute({ + serverLoader() { + return 1; + }, + async clientLoader({ serverLoader }) { + let serverData = await serverLoader(); + expectEqual(true); + return 2 as const; + }, + clientLoaderHydrate: true, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: server + client + hydrate + hydratefallback -> client +defineRoute({ + serverLoader() { + return 1; + }, + async clientLoader({ serverLoader }) { + let serverData = await serverLoader(); + expectEqual(true); + return 2 as const; + }, + clientLoaderHydrate: true, + HydrateFallback() { + return null; + }, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: client + hydrate + hydratefallback -> client +defineRoute({ + async clientLoader({ serverLoader }) { + expectEqual(true); + return 2 as const; + }, + clientLoaderHydrate: true, + HydrateFallback() { + return null; + }, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// Loader data: client + hydrate + -> client +defineRoute({ + async clientLoader({ serverLoader }) { + expectEqual(true); + return 2 as const; + }, + clientLoaderHydrate: true, + meta({ loaderData }) { + expectEqual(true); + return []; + }, + Component({ loaderData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ loaderData }) { + expectEqual(true); + return null; + }, +}); + +// action: neither, server, client, both + +// Action data: no actions -> undefined +defineRoute({ + Component({ actionData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ actionData }) { + expectEqual(true); + return null; + }, +}); + +// Action data: server -> server +defineRoute({ + serverAction() { + return 1; + }, + Component({ actionData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ actionData }) { + expectEqual(true); + return null; + }, +}); + +// Action data: client -> client +defineRoute({ + clientAction({ serverAction }) { + expectEqual(true); + return 2; + }, + Component({ actionData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ actionData }) { + expectEqual(true); + return null; + }, +}); + +// TODO: should it be `server | client` instead? +// Action data: server + client -> client +defineRoute({ + serverAction() { + return 1; + }, + clientAction() { + return 2; + }, + Component({ actionData }) { + expectEqual(true); + return null; + }, + ErrorBoundary({ actionData }) { + expectEqual(true); + return null; + }, +}); diff --git a/packages/react-router/lib/router/define-route.ts b/packages/react-router/lib/router/define-route.ts index 06b17b1449..a532303e05 100644 --- a/packages/react-router/lib/router/define-route.ts +++ b/packages/react-router/lib/router/define-route.ts @@ -1,8 +1,18 @@ import type { ReactNode } from "react"; +import type { DataRouteMatch } from "../context"; +import type { MetaDescriptor, RouteHandle } from "../dom/ssr/routeModules"; +import type { Location } from "./history"; +import type { UIMatch } from "./utils"; +import type { LinkDescriptor } from "../dom/ssr/links"; -interface Context {} // TODO: AppLoadContext? +// TODO: return interfaces for `Data` +// TODO: HMR +// TODO: allow widest type (branded type: NOT_SET) + +interface Context {} // TODO: AppLoadContext type MaybePromise = T | Promise; +type Pretty = { [K in keyof T]: T[K] } & {}; type Serializable = | undefined @@ -22,6 +32,10 @@ type Serializable = | Set | Promise; +type Data = MaybePromise< + Exclude> +>; + export type ResponseStub = { status: number | undefined; headers: Headers; @@ -31,40 +45,288 @@ export type ResponseStub = { type LoaderArgs = { context: Context; request: Request; - params: Record; + params: Params; response: ResponseStub; }; -export type Loader = ( - args: LoaderArgs -) => MaybePromise; // action type ActionArgs = { context: Context; request: Request; - params: Record; + params: Params; response: ResponseStub; }; -export type Action = ( - args: ActionArgs -) => MaybePromise; -type Component

> = (args: { - params: string extends P ? Record : Record; - data: Awaited>; +// prettier-ignore +type Params = Pretty< + & {[key: string]: string | undefined} + & {[K in Param as K extends `${string}?` ? never : K]: string} + & {[K in Param as K extends `${infer P}?` ? P : never]?: string} +> + +type IsDefined = undefined extends T ? false : true; + +// prettier-ignore +type _LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate extends boolean, + HydrateFallback, +> = Awaited< + [IsDefined, ClientLoaderHydrate] extends [true, true] ? + IsDefined extends true ? ClientLoaderData : + undefined + : + [IsDefined, IsDefined] extends [true, true] ? ServerLoaderData | ClientLoaderData : + IsDefined extends true ? + IsDefined extends true ? ClientLoaderData : + ClientLoaderData | undefined + : + IsDefined extends true ? ServerLoaderData : + undefined +> + +type LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate extends boolean, + HydrateFallback +> = _LoaderData< + Awaited, + Awaited, + ClientLoaderHydrate, + HydrateFallback +>; + +// prettier-ignore +type ActionData = Awaited< + IsDefined extends true ? ClientActionData : + IsDefined extends true ? ServerActionData : + undefined +> + +type HydrateFallbackComponent = (args: { + params: Params; }) => ReactNode; -// loader -> data for component -// loader -> serverLoader for clientLoader -> data for component -// TODO: clientLoader and all the other route module export APIs (meta, handle, ErrorBoundary, etc.) -// TODO: `handle`, HydrateFallback, shouldRevalidate, Layout, meta, links -export const defineRoute = < - const P extends string, - L extends Loader

, - A extends Action

->(route: { - params?: P[]; - loader?: L; - action?: A; - Component?: Component, NoInfer>; -}) => route; +type Route< + Param extends string, + ServerLoaderData extends Data | undefined, + ClientLoaderData extends Data | undefined, + ClientLoaderHydrate extends boolean, + HydrateFallback extends HydrateFallbackComponent | undefined, + ServerActionData extends Data | undefined, + ClientActionData extends Data | undefined +> = { + params?: Param[]; + links?: (args: { params: Params }) => LinkDescriptor[]; + HydrateFallback?: HydrateFallback; + + serverLoader?: (args: LoaderArgs) => ServerLoaderData; + clientLoader?: ( + args: LoaderArgs & { + serverLoader: IsDefined extends true + ? () => Promise> + : undefined; + } + ) => ClientLoaderData; + clientLoaderHydrate?: ClientLoaderHydrate; + + serverAction?: (args: ActionArgs) => ServerActionData; + clientAction?: ( + args: ActionArgs & { + serverAction: IsDefined extends true + ? () => Promise> + : undefined; + } + ) => ClientActionData; + + meta?: (args: { + params: Params; + location: Location; + error: unknown; + loaderData?: LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback + >; + matches?: Array; + }) => MetaDescriptor[]; + + Component?: (args: { + params: Params; + loaderData: LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback + >; + actionData?: ActionData; + }) => ReactNode; + ErrorBoundary?: (args: { + params: Params; + error: unknown; + loaderData?: LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback + >; + actionData?: ActionData; + }) => ReactNode; + + handle?: unknown; +}; + +export function defineRoute< + const Param extends string, + ServerLoaderData extends Data | undefined, + ClientLoaderData extends Data | undefined, + ClientLoaderHydrate extends boolean, + HydrateFallback extends HydrateFallbackComponent | undefined, + ServerActionData extends Data | undefined, + ClientActionData extends Data | undefined, + T +>( + route: T & + Route< + Param, + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback, + ServerActionData, + ClientActionData + > +): T { + return route; +} + +export function defineRootRoute< + const Param extends string, + ServerLoaderData extends Data | undefined, + ClientLoaderData extends Data | undefined, + ClientLoaderHydrate extends boolean, + HydrateFallback extends HydrateFallbackComponent | undefined, + ServerActionData extends Data | undefined, + ClientActionData extends Data | undefined, + T +>( + route: T & + Route< + Param, + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback, + ServerActionData, + ClientActionData + > & { + Layout: (args: { + params: Params; + error: unknown; + loaderData?: LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback + >; + actionData?: ActionData; + }) => ReactNode; + } +): T { + return route; +} + +type MetaMatch = { + id: RouteId; + pathname: DataRouteMatch["pathname"]; + data: Data; + handle?: RouteHandle; + params: DataRouteMatch["params"]; + meta: MetaDescriptor[]; + error?: unknown; +}; + +export type MetaMatches< + Routes extends Record> +> = Pretty< + Array< + Pretty< + { + [K in keyof Routes]: MetaMatch< + Exclude, + LoaderDataFromRoute + >; + }[keyof Routes] + > + > +>; + +type LoaderDataFromRoute = R extends Route< + any, + infer ServerLoaderData, + infer ClientLoaderData, + infer ClientLoaderHydrate, + infer HydrateFallback, + any, + any +> + ? LoaderData< + ServerLoaderData, + ClientLoaderData, + ClientLoaderHydrate, + HydrateFallback + > + : never; + +export type Matches< + Routes extends Record> +> = Pretty< + Array< + Pretty< + { + [K in keyof Routes]: { id: K } & UIMatch< + LoaderDataFromRoute + >; + }[keyof Routes] + > + > +>; + +let route1 = defineRoute({ + serverLoader: () => ({ hello: "world", project: "cool" }), +}); +let route2 = defineRoute({ + serverLoader: () => ({ goodbye: "planet", project: "sad" }), +}); + +type X = MetaMatches<{ + route1: typeof route1; + route2: typeof route2; +}>; + +// defineRoute({ +// meta({ matches }) { +// let typedMatches = matches as MetaMatches<{ +// route1: typeof route1; +// route2: typeof route2; +// }>; +// let project = typedMatches.find((match) => match.id === "route1")?.data +// .project; +// return []; +// }, +// Component() { +// let matches: Matches<{ +// route1: typeof route1; +// route2: typeof route2; +// }> = {} as any; +// console.log(matches); +// let match = matches[0]; +// if (match.id === "route1") { +// let x = match.data.hello; +// } +// return ""; +// }, +// });