Skip to content

Commit

Permalink
Fix remix-routes not transforming source maps (#8970)
Browse files Browse the repository at this point in the history
Co-authored-by: Pedro Cattori <pcattori@gmail.com>
  • Loading branch information
IgnusG and pcattori committed Mar 26, 2024
1 parent 82f50ac commit 3976575
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 43 deletions.
5 changes: 5 additions & 0 deletions .changeset/spicy-ghosts-watch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Vite: added sourcemap support for transformed routes
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@
- IAmLuisJ
- iamzee
- ianduvall
- IgnusG
- ikarus-akhil
- illright
- imzshh
Expand Down
11 changes: 7 additions & 4 deletions packages/remix-dev/vite/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1623,10 +1623,13 @@ export const remixVitePlugin: RemixVitePlugin = (remixUserConfig = {}) => {
}
}

return {
code: removeExports(code, SERVER_ONLY_ROUTE_EXPORTS),
map: null,
};
let [filepath] = id.split("?");

return removeExports(code, SERVER_ONLY_ROUTE_EXPORTS, {
sourceMaps: true,
filename: id,
sourceFileName: filepath,
});
},
},
{
Expand Down
70 changes: 35 additions & 35 deletions packages/remix-dev/vite/remove-exports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = () => {};
export const clientExport_2 = () => {};"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("arrow function with dependencies", () => {
Expand All @@ -34,21 +34,21 @@ describe("removeExports", () => {
export const serverExport_1 = () => serverUtil()
export const serverExport_2 = () => serverUtil()
export const clientExport_1 = () => clientUtil()
export const clientExport_2 = () => clientUtil()
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientLib } from 'client-lib';
import { sharedLib } from 'shared-lib';
const sharedUtil = () => sharedLib();
const clientUtil = () => sharedUtil(clientLib());
export const clientExport_1 = () => clientUtil();
export const clientExport_2 = () => clientUtil();"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("function statement", () => {
Expand All @@ -62,11 +62,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export function clientExport_1() {}
export function clientExport_2() {}"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("function statement with dependencies", () => {
Expand All @@ -90,7 +90,7 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientLib } from 'client-lib';
import { sharedLib } from 'shared-lib';
function sharedUtil() {
Expand All @@ -106,7 +106,7 @@ describe("removeExports", () => {
return clientUtil();
}"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("object", () => {
Expand All @@ -120,11 +120,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = {};
export const clientExport_2 = {};"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("object with dependencies", () => {
Expand All @@ -148,7 +148,7 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientLib } from 'client-lib';
import { sharedLib } from 'shared-lib';
const sharedUtil = () => sharedLib();
Expand All @@ -160,7 +160,7 @@ describe("removeExports", () => {
value: clientUtil()
};"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("function call", () => {
Expand All @@ -174,11 +174,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = globalFunction();
export const clientExport_2 = globalFunction();"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("function call with dependencies", () => {
Expand All @@ -202,15 +202,15 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientLib } from 'client-lib';
import { sharedLib } from 'shared-lib';
const sharedUtil = () => sharedLib();
const clientUtil = () => sharedUtil(clientLib());
export const clientExport_1 = clientUtil();
export const clientExport_2 = clientUtil();"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("iife", () => {
Expand All @@ -224,11 +224,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = (() => {})();
export const clientExport_2 = (() => {})();"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("iife with dependencies", () => {
Expand All @@ -252,15 +252,15 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientLib } from 'client-lib';
import { sharedLib } from 'shared-lib';
const sharedUtil = () => sharedLib();
const clientUtil = () => sharedUtil(clientLib());
export const clientExport_1 = (() => clientUtil())();
export const clientExport_2 = (() => clientUtil())();"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("re-export", () => {
Expand All @@ -274,11 +274,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export { clientExport_1 } from './client/1';
export { clientExport_2 } from './client/2';"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("re-export multiple", () => {
Expand All @@ -290,10 +290,10 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(
expect(result.code).toMatchInlineSnapshot(
"\"export { clientExport_1, clientExport_2 } from './client';\""
);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("re-export manual", () => {
Expand All @@ -312,13 +312,13 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"import { clientExport_1 } from './client/1';
import { clientExport_2 } from './client/2';
export { clientExport_1 };
export { clientExport_2 };"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("number", () => {
Expand All @@ -332,11 +332,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = 123;
export const clientExport_2 = 123;"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("string", () => {
Expand All @@ -350,11 +350,11 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = 'string';
export const clientExport_2 = 'string';"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("string reference", () => {
Expand All @@ -371,12 +371,12 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"const CLIENT_STRING = 'CLIENT_STRING';
export const clientExport_1 = CLIENT_STRING;
export const clientExport_2 = CLIENT_STRING;"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});

test("null", () => {
Expand All @@ -390,10 +390,10 @@ describe("removeExports", () => {
`,
["serverExport_1", "serverExport_2"]
);
expect(result).toMatchInlineSnapshot(`
expect(result.code).toMatchInlineSnapshot(`
"export const clientExport_1 = null;
export const clientExport_2 = null;"
`);
expect(result).not.toMatch(/server/i);
expect(result.code).not.toMatch(/server/i);
});
});
13 changes: 9 additions & 4 deletions packages/remix-dev/vite/remove-exports.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Adapted from https://github.com/egoist/babel-plugin-eliminator/blob/d29859396b7708b7f7abbacdd951cbbc80902f00/src/index.ts
// Which was originally adapted from https://github.com/vercel/next.js/blob/574fe0b582d5cc1b13663121fd47a3d82deaaa17/packages/next/build/babel/plugins/next-ssg-transform.ts
import type { GeneratorOptions } from "@babel/generator";

import {
type BabelTypes,
type NodePath,
Expand Down Expand Up @@ -61,9 +63,12 @@ function isIdentifierReferenced(
return false;
}

export const removeExports = (source: string, exportsToRemove: string[]) => {
export const removeExports = (
source: string,
exportsToRemove: string[],
generateOptions: GeneratorOptions = {}
) => {
let document = parse(source, { sourceType: "module" });
let generateCode = () => generate(document).code;

let referencedIdentifiers = new Set<NodePath<BabelTypes.Identifier>>();
let removedExports = new Set<string>();
Expand Down Expand Up @@ -213,7 +218,7 @@ export const removeExports = (source: string, exportsToRemove: string[]) => {
if (removedExports.size === 0) {
// No server-specific exports found so there's
// no need to remove unused references
return generateCode();
return generate(document, generateOptions);
}

let referencesRemovedInThisPass: number;
Expand Down Expand Up @@ -359,5 +364,5 @@ export const removeExports = (source: string, exportsToRemove: string[]) => {
});
} while (referencesRemovedInThisPass);

return generateCode();
return generate(document, generateOptions);
};

0 comments on commit 3976575

Please sign in to comment.