From 5120f8261f9bfd93d7e42a29656bec31d4bfbb07 Mon Sep 17 00:00:00 2001 From: Pedro Cattori Date: Fri, 1 Dec 2023 02:05:18 -0500 Subject: [PATCH] feat(vite): strict route exports (#8171) --- .changeset/grumpy-cats-roll.md | 11 ++++ docs/future/vite.md | 68 +++++++++++++++++++++++++ integration/vite-route-exports-test.ts | 70 ++++++++++++++++++++++++++ packages/remix-dev/vite/plugin.ts | 31 +++++++++++- 4 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 .changeset/grumpy-cats-roll.md create mode 100644 integration/vite-route-exports-test.ts diff --git a/.changeset/grumpy-cats-roll.md b/.changeset/grumpy-cats-roll.md new file mode 100644 index 00000000000..28e1cf06b84 --- /dev/null +++ b/.changeset/grumpy-cats-roll.md @@ -0,0 +1,11 @@ +--- +"@remix-run/dev": minor +--- + +Vite: Strict route exports + +With Vite, Remix gets stricter about which exports are allowed from your route modules. +Previously, the Remix compiler would allow any export from routes. +While this was convenient, it was also a common source of bugs that were hard to track down because they only surfaced at runtime. + +For more, see https://remix.run/docs/en/main/future/vite#strict-route-exports diff --git a/docs/future/vite.md b/docs/future/vite.md index 7cf7bc47f41..63ad90a5b1f 100644 --- a/docs/future/vite.md +++ b/docs/future/vite.md @@ -634,6 +634,74 @@ const posts = import.meta.glob("./posts/*.mdx", { }); ``` +#### Strict route exports + +With Vite, Remix gets stricter about which exports are allowed from your route modules. + +Previously, Remix allowed user-defined exports from routes. +The Remix compiler would then rely on treeshaking to remove any code only intended for use on the server from the client bundle. + +In contrast, Vite processes each module in isolation during development, so cross-module treeshaking is not possible. +You should already be separating server-only code into `.server` files or directories, so treeshaking isn't needed for those modules. +But routes are a special case since they intentionally blend client and server code. +Remix knows that exports like `loader`, `action`, `headers`, etc. are server-only, so it can safely remove them from the client bundle. +But there's no way to know when looking at a single route module in isolation whether user-defined exports are server-only. +That's why Remix's Vite plugin is stricter about which exports are allowed from your route modules. + +In fact, we'd rather not rely on treeshaking for correctness at all. +If tomorrow you or your coworker accidentally imports something you _thought_ was client-safe, +treeshaking will no longer exclude that from your client bundle and you might end up with server code in your app! +Treeshaking is designed as a pure optimization, so relying on it for correctness is brittle. + +So instead of treeshaking, its better to be explicit about what code is client-safe and what code is server-only. +For route modules, that means only exporting Remix route exports. +For anything else, put it in a separate module and use a `.server` file or directory when needed. + +Ultimately, Route exports are Remix API. +Think of a Remix route module like a function and the exports like named arguments to the function. + +```ts +// Not real API, just a mental model +let route = createRoute({ loader, mySuperCoolThing }); +// ^^^^^^^^^^^^^^^^ +// Object literal may only specify known properties, and 'mySuperCoolThing' does not exist in type 'RemixRoute' +``` + +Just like how you shouldn't pass unexpected named arguments to a function, you shouldn't create unexpected exports from a route module. +The result is that Remix is simpler and more predictable. +In short, Vite made us eat our veggies, but turns out they were delicious all along! + +👉 **Move any user-defined route exports to a separate module** + +For example, here's a route with a user-defined export called `mySuperCoolThing`: + +```ts filename=app/routes/super-cool.tsx +// ❌ This isn't a Remix-specific route export, just something I made up +export const mySuperCoolThing = + "Some value I wanted to colocate with my route!"; + +// ✅ This is a valid Remix route export, so it's fine +export const loader = () => {}; + +// ✅ This is also a valid Remix route export +export default function SuperCool() {} +``` + +One option is to colocate your route and related utilities in the same directory if your routing convention allows it. +For example, with the default route convention in v2: + +```ts filename=app/routes/super-cool/route.tsx +export const loader = () => {}; + +export default function SuperCool() {} +``` + +```ts filename=app/routes/super-cool/utils.ts +// If this was server-only code, I'd rename this file to "utils.server.ts" +export const mySuperCoolThing = + "Some value I wanted to colocate with my route!"; +``` + ## Troubleshooting Check out the [known issues with the Remix Vite plugin on GitHub][issues-vite] before filing a new bug report! diff --git a/integration/vite-route-exports-test.ts b/integration/vite-route-exports-test.ts new file mode 100644 index 00000000000..80e4673e4ef --- /dev/null +++ b/integration/vite-route-exports-test.ts @@ -0,0 +1,70 @@ +import { test, expect } from "@playwright/test"; + +import { createProject, viteBuild } from "./helpers/vite.js"; + +test("Vite / invalid route exports / expected build error", async () => { + let cwd = await createProject({ + "app/routes/fail-non-remix-exports.tsx": String.raw` + // Remix exports + export const ErrorBoundary = () => {} + export const action = () => {} + export const handle = () => {} + export const headers = () => {} + export const links = () => {} + export const loader = () => {} + export const meta = () => {} + export const shouldRevalidate = () => {} + export default function() {} + + // Non-Remix exports + export const invalid1 = 1; + export const invalid2 = 2; + `, + }); + let client = viteBuild({ cwd })[0]; + let stderr = client.stderr.toString("utf8"); + expect(stderr).toMatch( + "2 invalid route exports in `routes/fail-non-remix-exports.tsx`:\n - `invalid1`\n - `invalid2`" + ); + expect(stderr).toMatch( + "See https://remix.run/docs/en/main/future/vite#strict-route-exports" + ); +}); + +test("Vite / invalid route exports / ignore in mdx", async () => { + let cwd = await createProject({ + "vite.config.ts": String.raw` + import { defineConfig } from "vite"; + import { unstable_vitePlugin as remix } from "@remix-run/dev"; + import mdx from "@mdx-js/rollup"; + + export default defineConfig({ + plugins: [ + remix(), + mdx(), + ], + }); + `, + "app/routes/pass-non-remix-exports-in-mdx.mdx": String.raw` + // Remix exports + export const ErrorBoundary = () => {} + export const action = () => {} + export const handle = () => {} + export const headers = () => {} + export const links = () => {} + export const loader = () => {} + export const meta = () => {} + export const shouldRevalidate = () => {} + export default function() {} + + // Non-Remix exports + export const invalid1 = 1; + export const invalid2 = 2; + + # Hello World + `, + }); + let [client, server] = viteBuild({ cwd }); + expect(client.status).toBe(0); + expect(server.status).toBe(0); +}); diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 844efaa7b4d..a0a027a3f16 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -47,6 +47,18 @@ const supportedRemixConfigKeys = [ type SupportedRemixConfigKey = typeof supportedRemixConfigKeys[number]; type SupportedRemixConfig = Pick; +const ROUTE_EXPORTS = new Set([ + "ErrorBoundary", + "action", + "default", // component + "handle", + "headers", + "links", + "loader", + "meta", + "shouldRevalidate", +]); + // We need to provide different JSDoc comments in some cases due to differences // between the Remix config and the Vite plugin. type RemixConfigJsdocOverrides = { @@ -940,7 +952,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { }, }, { - name: "remix-remove-server-exports", + name: "remix-route-exports", enforce: "post", // Ensure we're operating on the transformed code to support MDX etc. async transform(code, id, options) { if (options?.ssr) return; @@ -950,6 +962,23 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { let route = getRoute(pluginConfig, id); if (!route) return; + // check the exports, fail if unknown exists, unless id ends with .mdx + let nonRemixExports = esModuleLexer(code)[1] + .map((exp) => exp.n) + .filter((exp) => !ROUTE_EXPORTS.has(exp)); + if (nonRemixExports.length > 0 && !id.endsWith(".mdx")) { + let message = [ + `${nonRemixExports.length} invalid route export${ + nonRemixExports.length > 1 ? "s" : "" + } in \`${route.file}\`:`, + ...nonRemixExports.map((exp) => ` - \`${exp}\``), + "", + "See https://remix.run/docs/en/main/future/vite#strict-route-exports", + "", + ].join("\n"); + throw Error(message); + } + let serverExports = ["loader", "action", "headers"]; return {