Skip to content

Commit

Permalink
fix (remix-serve): Fix source map loading (#8174)
Browse files Browse the repository at this point in the history 
Co-authored-by: Matt Brophy <[email protected]>
Co-authored-by: Pedro Cattori <[email protected]>
  • Loading branch information
@kiliman @brophdawg11 @pcattori
3 people authored Dec 4, 2023
1 parent 4222e71 commit 999b565
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/sour-stingrays-roll.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/serve": patch
---

Fix source map loading when file has `?t=timestamp` suffix (rebuilds)
14 changes: 13 additions & 1 deletion packages/remix-serve/cli.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,19 @@ import getPort from "get-port";

process.env.NODE_ENV = process.env.NODE_ENV ?? "production";

sourceMapSupport.install();
sourceMapSupport.install({
retrieveSourceMap: function (source) {
// get source file without the `file://` prefix or `?t=...` suffix
let match = source.match(/^file:\/\/(.*)\?t=[.\d]+$/);
if (match) {
return {
url: source,
map: fs.readFileSync(`${match[1]}.map`, "utf8"),
};
}
return null;
},
});
installGlobals();

run();
Expand Down
14 changes: 13 additions & 1 deletion templates/express/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,19 @@ import express from "express";
import morgan from "morgan";
import sourceMapSupport from "source-map-support";

sourceMapSupport.install();
sourceMapSupport.install({
retrieveSourceMap: function (source) {
// get source file without the `file://` prefix or `?t=...` suffix
const match = source.match(/^file:\/\/(.*)\?t=[.\d]+$/);
if (match) {
return {
url: source,
map: fs.readFileSync(`${match[1]}.map`, "utf8"),
};
}
return null;
},
});
installGlobals();

/** @typedef {import('@remix-run/node').ServerBuild} ServerBuild */
Expand Down

2 comments on commit 999b565

@omelhus
Copy link

@omelhus omelhus commented on 999b565 Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: just saw that this was fixed 20h ago and is just awaiting the next release. :-)

Hey friends,

This commit broke loading source maps on windows. The file reference on Windows gets a leading \ that breaks loading it. I've manually changed the code locally and this fix works for me:

let file_path = path.normalize(match[1]);
if(process.platform === "win32" && file_path.startsWith("\\")) {
  file_path = file_path.slice(1);
}
return {
  url: source,
  map: fs__default["default"].readFileSync(`${file_path}.map`, "utf8")
};

@kiliman
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry about that. I should have tested on Windows, especially when it is path-related.

Please sign in to comment.