Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve route module imports back to virtual #6098

Merged
merged 7 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mdx-automatic-jsx-runtime.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Use the "automatic" JSX runtime when processing MDX files.
5 changes: 5 additions & 0 deletions .changeset/unstable-dev-resolve-route-module-imports.md
Original file line number Diff line number Diff line change
@@ -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.
219 changes: 173 additions & 46 deletions integration/shared-route-imports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('✅')");
});
});
30 changes: 24 additions & 6 deletions packages/remix-dev/compiler/assets/js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }),
Expand Down
Loading