fix: revert browserRouteModulesPlugin

fix: put new browserRouteModulesPlugin behind `config.future.unstable_dev`
remix-run · Mar 8, 2023 · 225716c · 225716c
1 parent b74dbae
commit 225716c
Show file tree

Hide file tree

Showing 5 changed files with 255 additions and 102 deletions.
diff --git a/.changeset/fix-x-route-imports.md b/.changeset/fix-x-route-imports.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+fix issue with x-route imports creating multiple entries in the module graph
diff --git a/integration/shared-route-imports-test.ts b/integration/shared-route-imports-test.ts
@@ -0,0 +1,60 @@
+import { test, expect } from "@playwright/test";
+
+import { PlaywrightFixture } from "./helpers/playwright-fixture";
+import type { Fixture, AppFixture } from "./helpers/create-fixture";
+import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+
+let fixture: Fixture;
+let appFixture: AppFixture;
+
+test.beforeAll(async () => {
+  fixture = await createFixture({
+    future: { v2_routeConvention: true },
+    files: {
+      "app/routes/parent.jsx": js`
+        import { createContext, useContext } from "react";
+        import { Outlet } from "@remix-run/react";
+
+        const ParentContext = createContext("❌");
+
+        export function useParentContext() {
+          return useContext(ParentContext);
+        }
+
+        export default function Index() {
+          return (
+            <ParentContext.Provider value="✅">
+              <Outlet />
+            </ParentContext.Provider>
+          )
+        }
+      `,
+
+      "app/routes/parent.child.jsx": js`
+        import { useParentContext } from "./parent";
+
+        export default function Index() {
+          return <p>{useParentContext()}</p>;
+        }
+      `,
+    },
+  });
+
+  appFixture = await createAppFixture(fixture);
+});
+
+test.afterAll(() => {
+  appFixture.close();
+});
+
+test("[description of what you expect it to do]", async ({ page }) => {
+  let app = new PlaywrightFixture(appFixture, page);
+  // If you need to test interactivity use the `app`
+  await app.goto("/parent/child", true);
+
+  await page.waitForSelector("p:has-text('✅')");
+});
+
+////////////////////////////////////////////////////////////////////////////////
+// 💿 Finally, push your changes to your fork of Remix and open a pull request!
+////////////////////////////////////////////////////////////////////////////////
diff --git a/packages/remix-dev/compiler/compileBrowser.ts b/packages/remix-dev/compiler/compileBrowser.ts
@@ -14,6 +14,7 @@ import { getAppDependencies } from "./dependencies";
 import { loaders } from "./loaders";
 import type { CompileOptions } from "./options";
 import { browserRouteModulesPlugin } from "./plugins/browserRouteModulesPlugin";
+import { browserRouteModulesPlugin as browserRouteModulesPlugin_v2 } from "./plugins/browserRouteModulesPlugin_v2";
 import { cssFilePlugin } from "./plugins/cssFilePlugin";
 import { deprecatedRemixPackagePlugin } from "./plugins/deprecatedRemixPackagePlugin";
 import { emptyModulesPlugin } from "./plugins/emptyModulesPlugin";
@@ -122,7 +123,9 @@ const createEsbuildConfig = (
     cssFilePlugin({ config, options }),
     urlImportsPlugin(),
     mdxPlugin(config),
-    browserRouteModulesPlugin(config, /\?browser$/, onLoader, mode),
+    config.future.unstable_dev
+      ? browserRouteModulesPlugin_v2(config, /\?browser$/, onLoader, mode)
+      : browserRouteModulesPlugin(config, /\?browser$/),
     emptyModulesPlugin(config, /\.server(\.[jt]sx?)?$/),
     NodeModulesPolyfillPlugin(),
   ].filter(isNotNull);

diff --git a/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts b/packages/remix-dev/compiler/plugins/browserRouteModulesPlugin.ts
@@ -1,83 +1,28 @@
-import * as fs from "node:fs";
-import * as path from "node:path";
 import type esbuild from "esbuild";
-import generate from "@babel/generator";
 
 import type { RemixConfig } from "../../config";
-import invariant from "../../invariant";
-import * as Transform from "../../transform";
-import type { CompileOptions } from "../options";
-import { getLoaderForFile } from "../loaders";
 import { getRouteModuleExports } from "../routeExports";
-import { applyHMR } from "./hmrPlugin";
-
-const serverOnlyExports = new Set(["action", "loader"]);
-
-let removeServerExports = (onLoader: (loader: string) => void) =>
-  Transform.create(({ types: t }) => {
-    return {
-      visitor: {
-        ExportNamedDeclaration: (path) => {
-          let { node } = path;
-          if (node.source) {
-            let specifiers = node.specifiers.filter(({ exported }) => {
-              let name = t.isIdentifier(exported)
-                ? exported.name
-                : exported.value;
-              return !serverOnlyExports.has(name);
-            });
-            if (specifiers.length === node.specifiers.length) return;
-            if (specifiers.length === 0) return path.remove();
-            path.replaceWith(
-              t.exportNamedDeclaration(
-                node.declaration,
-                specifiers,
-                node.source
-              )
-            );
-          }
-          if (t.isFunctionDeclaration(node.declaration)) {
-            let name = node.declaration.id?.name;
-            if (!name) return;
-            if (name === "loader") {
-              let { code } = generate(node);
-              onLoader(code);
-            }
-            if (serverOnlyExports.has(name)) return path.remove();
-          }
-          if (t.isVariableDeclaration(node.declaration)) {
-            let declarations = node.declaration.declarations.filter((d) => {
-              let name = t.isIdentifier(d.id) ? d.id.name : undefined;
-              if (!name) return false;
-              if (name === "loader") {
-                let { code } = generate(node);
-                onLoader(code);
-              }
-              return !serverOnlyExports.has(name);
-            });
-            if (declarations.length === 0) return path.remove();
-            if (declarations.length === node.declaration.declarations.length)
-              return;
-            path.replaceWith(
-              t.variableDeclaration(node.declaration.kind, declarations)
-            );
-          }
-        },
-      },
-    };
-  });
+import invariant from "../../invariant";
 
 type Route = RemixConfig["routes"][string];
 
+const browserSafeRouteExports: { [name: string]: boolean } = {
+  CatchBoundary: true,
+  ErrorBoundary: true,
+  default: true,
+  handle: true,
+  links: true,
+  meta: true,
+  shouldRevalidate: true,
+};
+
 /**
  * This plugin loads route modules for the browser build, using module shims
  * that re-export only the route module exports that are safe for the browser.
  */
 export function browserRouteModulesPlugin(
   config: RemixConfig,
-  suffixMatcher: RegExp,
-  onLoader: (filename: string, code: string) => void,
-  mode: CompileOptions["mode"]
+  suffixMatcher: RegExp
 ): esbuild.Plugin {
   return {
     name: "browser-route-modules",
@@ -90,6 +35,7 @@ export function browserRouteModulesPlugin(
         },
         new Map()
       );
+
       build.onResolve({ filter: suffixMatcher }, (args) => {
         return {
           path: args.path,
@@ -100,54 +46,40 @@ export function browserRouteModulesPlugin(
       build.onLoad(
         { filter: suffixMatcher, namespace: "browser-route-module" },
         async (args) => {
+          let theExports;
           let file = args.path.replace(suffixMatcher, "");
+          let route = routesByFile.get(file);
 
-          if (/\.mdx?$/.test(file)) {
-            let route = routesByFile.get(file);
+          try {
             invariant(route, `Cannot get route by path: ${args.path}`);
 
-            let theExports = await getRouteModuleExports(config, route.id);
-            let contents = "module.exports = {};";
-            if (theExports.length !== 0) {
-              let spec = `{ ${theExports.join(", ")} }`;
-              contents = `export ${spec} from ${JSON.stringify(`./${file}`)};`;
-            }
-
+            theExports = (await getRouteModuleExports(config, route.id)).filter(
+              (ex) => !!browserSafeRouteExports[ex]
+            );
+          } catch (error: any) {
             return {
-              contents,
-              resolveDir: config.appDirectory,
-              loader: "js",
+              errors: [
+                {
+                  text: error.message,
+                  pluginName: "browser-route-module",
+                },
+              ],
             };
           }
 
-          let routeFile = path.join(config.appDirectory, file);
-
-          let sourceCode = fs.readFileSync(routeFile, "utf8");
-
-          let transform = removeServerExports((loader: string) =>
-            onLoader(routeFile, loader)
-          );
-          let contents = transform(sourceCode, routeFile);
-
-          if (mode === "development" && config.future.unstable_dev) {
-            contents = await applyHMR(
-              contents,
-              {
-                ...args,
-                path: routeFile,
-              },
-              config,
-              !!build.initialOptions.sourcemap
-            );
+          let contents = "module.exports = {};";
+          if (theExports.length !== 0) {
+            let spec = `{ ${theExports.join(", ")} }`;
+            contents = `export ${spec} from ${JSON.stringify(`./${file}`)};`;
           }
 
           return {
             contents,
-            loader: getLoaderForFile(routeFile),
-            resolveDir: path.dirname(routeFile),
+            resolveDir: config.appDirectory,
+            loader: "js",
           };
         }
       );
     },
   };
-}
+}