fix: resolve route module imports back to virtual (#6098)

.changeset/mdx-automatic-jsx-runtime.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Use the "automatic" JSX runtime when processing MDX files.

.changeset/unstable-dev-resolve-route-module-imports.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Resolve imports from route modules across the graph back to the virtual module created by the v2 routes plugin. This fixes issues where we would duplicate portions of route modules that were imported.

integration/shared-route-imports-test.ts

@@ -7,54 +7,181 @@ 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.describe("v1 compiler", () => {
+  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>
+            )
+          }
+        `,
 
-test.afterAll(() => {
-  appFixture.close();
-});
+        "app/routes/parent.child.jsx": js`
+          import { useParentContext } from "./parent";
+  
+          export default function Index() {
+            return <p>{useParentContext()}</p>;
+          }
+        `,
+
+        "app/routes/markdown-parent.mdx": `import { createContext, useContext } from 'react';
+import { Outlet } from '@remix-run/react';
+
+export const ParentContext = createContext("❌");
+
+export function useParentContext() {
+  return useContext(ParentContext);
+}
+
+export function ParentProvider() {
+  return (
+    <ParentContext.Provider value="✅">
+      <Outlet />
+    </ParentContext.Provider>
+  );
+}
+
+<ParentProvider />
+`,
+        "app/routes/markdown-parent.child.mdx": `import { useParentContext } from "./markdown-parent.mdx";
+
+export function UseParentContext() {
+  return <p>{useParentContext()}</p>;
+}
+
+<UseParentContext />
+`,
+      },
+    });
 
-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);
+    appFixture = await createAppFixture(fixture);
+  });
+
+  test.afterAll(() => {
+    appFixture.close();
+  });
 
-  await page.waitForSelector("p:has-text('✅')");
+  test("should render context value from context provider", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/parent/child", true);
+
+    await page.waitForSelector("p:has-text('✅')");
+  });
+
+  test("should render context value from context provider exported from mdx", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/markdown-parent/child", true);
+
+    await page.waitForSelector("p:has-text('✅')");
+  });
 });
 
-////////////////////////////////////////////////////////////////////////////////
-// 💿 Finally, push your changes to your fork of Remix and open a pull request!
-////////////////////////////////////////////////////////////////////////////////
+test.describe("v2 compiler", () => {
+  test.beforeAll(async () => {
+    fixture = await createFixture({
+      future: { v2_routeConvention: true, unstable_dev: 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>;
+          }
+        `,
+
+        "app/routes/markdown-parent.mdx": `import { createContext, useContext } from 'react';
+import { Outlet } from '@remix-run/react';
+
+export const ParentContext = createContext("❌");
+
+export function useParentContext() {
+  return useContext(ParentContext);
+}
+
+export function ParentProvider() {
+  return (
+    <ParentContext.Provider value="✅">
+      <Outlet />
+    </ParentContext.Provider>
+  );
+}
+
+<ParentProvider />
+`,
+        "app/routes/markdown-parent.child.mdx": `import { useParentContext } from "./markdown-parent.mdx";
+
+export function UseParentContext() {
+  const value = useParentContext();
+  return (
+    <p>{value}</p>
+  );
+}
+
+<UseParentContext />
+`,
+      },
+    });
+
+    appFixture = await createAppFixture(fixture);
+  });
+
+  test.afterAll(() => {
+    appFixture.close();
+  });
+
+  test("should render context value from context provider", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/parent/child", true);
+
+    await page.waitForSelector("p:has-text('✅')");
+  });
+
+  test("should render context value from context provider exported from mdx", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/markdown-parent/child", true);
+
+    await page.waitForSelector("p:has-text('✅')");
+  });
+});

packages/remix-dev/compiler/assets/js.ts

@@ -78,11 +78,29 @@ const createEsbuildConfig = (
     "entry.client": ctx.config.entryClientFilePath,
   };
 
+  let routeModulePaths = new Map<string, string>();
   for (let id of Object.keys(ctx.config.routes)) {
-    // All route entry points are virtual modules that will be loaded by the
-    // browserEntryPointsPlugin. This allows us to tree-shake server-only code
-    // that we don't want to run in the browser (i.e. action & loader).
-    entryPoints[id] = ctx.config.routes[id].file + "?browser";
+    entryPoints[id] = ctx.config.routes[id].file;
+    if (ctx.config.future.unstable_dev) {
+      // In V2 we are doing AST transforms to remove server code, this means we
+      // have to re-map all route modules back to the same module in the graph
+      // otherwise we will have duplicate modules in the graph. We have to resolve
+      // the path as we get the relative for the entrypoint and absolute for imports
+      // from other modules.
+      routeModulePaths.set(
+        ctx.config.routes[id].file,
+        ctx.config.routes[id].file
+      );
+      routeModulePaths.set(
+        path.resolve(ctx.config.appDirectory, ctx.config.routes[id].file),
+        ctx.config.routes[id].file
+      );
+    } else {
+      // All route entry points are virtual modules that will be loaded by the
+      // browserEntryPointsPlugin. This allows us to tree-shake server-only code
+      // that we don't want to run in the browser (i.e. action & loader).
+      entryPoints[id] += "?browser";
+    }
   }
 
   let matchPath = ctx.config.tsconfigPath
@@ -105,10 +123,10 @@ const createEsbuildConfig = (
     cssFilePlugin(ctx),
     absoluteCssUrlsPlugin(),
     externalPlugin(/^https?:\/\//, { sideEffects: false }),
-    mdxPlugin(ctx),
     ctx.config.future.unstable_dev
-      ? browserRouteModulesPlugin_v2(ctx, /\?browser$/, onLoader)
+      ? browserRouteModulesPlugin_v2(ctx, routeModulePaths, onLoader)
       : browserRouteModulesPlugin(ctx, /\?browser$/),
+    mdxPlugin(ctx),
     emptyModulesPlugin(ctx, /\.server(\.[jt]sx?)?$/),
     NodeModulesPolyfillPlugin(),
     externalPlugin(/^node:.*/, { sideEffects: false }),