fix(remix-dev): fix sourcemap path replace in Cloudflare Pages build

[fabiankaestner]

Closes: #3768

• Docs
• Tests

Testing Strategy:

I built the remix project, and linked remix-dev:

yarn build
cd packages/remix-dev/
yarn link

Then I linked the remix-dev build in a cloudflare-workers project:

cd some-cloudflare-project/
yarn link @remix-run/dev
yarn dev

and verified that the sourcemaps work.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5d3e605

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[fabiankaestner]

Is there anything more I need to do to get this merged?

[MichaelDeBoey]

@fabiankaestner If all tests are passing, the only thing you can do is wait for somebody from the team to accept/decline this PR

[sudomf]

looking forward to see this merged. Cloudflare pages build is being generated with wrong source map url

[sudomf]

hey @kiliman . Is there anything you can do to get this PR merged ASAP? 👀

[kiliman]

@sudomf unfortunately, I don't have that kind of permission. One option is to use patch-package to unblock you while you wait. Let me know if you need help creating the patch.

[Ponjimon]

As a workaround, I already created a patch for my local repo:

diff --git a/dist/compiler/compilerServer.js b/dist/compiler/compilerServer.js
index 4d31adfdf0c70ea572c0f61d266d8f30ce0f579b..431a53b1902e9128f87d7eb78eebe6374628d318 100644
--- a/dist/compiler/compilerServer.js
+++ b/dist/compiler/compilerServer.js
@@ -137,7 +137,7 @@ async function writeServerBuildResult(config, outputFiles) {
     if (file.path.endsWith(".js")) {
       // fix sourceMappingURL to be relative to current path instead of /build
       let filename = file.path.substring(file.path.lastIndexOf(path__namespace.sep) + 1);
-      let escapedFilename = filename.replace(/\./g, "\\.");
+      let escapedFilename = filename.replace(/([.\[\]])/g, '\\$1');
       let pattern = `(//# sourceMappingURL=)(.*)${escapedFilename}`;
       let contents = Buffer.from(file.contents).toString("utf-8");
       contents = contents.replace(new RegExp(pattern), `$1${filename}`);

It doesn't use the PRs change though but rather @kiliman's suggestion :)

Note
I used pnpm's patch feature to auto-generate this diff, I'm unfamiliar with patch-package, so it may or may not work.

[xdivby0]

Since this 4 line change PR is still open for more than 2 months, should we rather lose hope that this will be merged at some point? Is there anything we as the community can do to help the maintainers get this sorted out quicker?

This is kind of a blocker since I have 0 confidence in my code because stack traces are useless should anything happen.

I will look if I can create a patch like others suggested probably.

[kiliman]

@xdivby0 Yeah, unfortunately, the Remix team is small and not helped by the recent Shopify layoffs. Plus, they also maintain React Router.

If something is a show-stopper, then you'll either need to work around the issue or create a patch. That's really the only way to move forward.

[xdivby0]

For everyone struggling if they should wait or just patch it themselves, do patch it yourself. I thought this is hard or takes a long time but I got it to work in 1 minute with @kiliman's recommendation of using patch-package, it's really simple.

(Also yes, this one line change was what made my source maps work again, thanks to you all)

[stephenkiers]

Looks good to me. Excited to see this merged. :)

[KrisBraun]

Note with @remix-run/[email protected], the patch is now

--- a/dist/compiler/server/write.js
+++ b/dist/compiler/server/write.js
@@ -42,7 +42,7 @@ async function write(config, outputFiles) {
     if (file.path.endsWith(".js") || file.path.endsWith(".mjs")) {
       // fix sourceMappingURL to be relative to current path instead of /build
       let filename = file.path.substring(file.path.lastIndexOf(path__namespace.sep) + 1);
-      let escapedFilename = filename.replace(/\./g, "\\.");
+      let escapedFilename = filename.replace(/([.\[\]])/g, '\\$1');
       let pattern = `(//# sourceMappingURL=)(.*)${escapedFilename}`;
       let contents = Buffer.from(file.contents).toString("utf-8");
       contents = contents.replace(new RegExp(pattern), `$1${filename}`);

[KrisBraun]

@Ponjimon @xdivby0 Did you need to do anything else to get sourcemaps to match the dynamic path? While [[path]].js ends with the line //# sourceMappingURL=[[path]].js.map, I see errors from the worker report in Sentry with paths like /functionsWorker-0.38181357489899037.js. How can I get these paths to match up?

Edit: I have the mapping between source and sourcemap working in Sentry using RewriteFrames. Unfortunately, the line numbers in the stack traces don't seem to line up.

[xdivby0]

@Ponjimon @xdivby0 Did you need to do anything else to get sourcemaps to match the dynamic path? While [[path]].js ends with the line //# sourceMappingURL=[[path]].js.map, I see errors from the worker report in Sentry with paths like /functionsWorker-0.38181357489899037.js. How can I get these paths to match up?

I haven't tried anything with sentry in general, maybe it has messed the error up trying to "source-map" it again (even though the error stack trace inside the handleError already works without sentries mapping).

Does it log correctly inside the workers log (when you live stream the logs and provoke an error or throw a artificially created one)? If yes, it's probably something that sentry does. If not, does it work in dev for you (I've only played with 1.17 on my own machine for now)

[zbynekwinkler]

and verified that the sourcemaps work.

How does one do that? I ask wrt #6702 since I'd like to help debugging this.

[KrisBraun]

My issue stemmed from Cloudflare applying a further bundling and minification step in the deploy that I didn't know about. After switching to upload those final artifacts, I'm in pretty good shape. For some reason, errors tend to map in Sentry against the line with the definition of the function throwing the exception, rather than the line throwing the exception itself. But other than that, things are working for me now.

[BrandonNoad]

Does anyone using this patch have source maps working for the stack property of the errors in handleError (https://remix.run/docs/en/main/file-conventions/entry.server#handleerror)?

@xdivby0 it sounds like you may have it working?

I've added source-map-support to prep for v2, and it isn't able to find my source maps (at least in my dev env). It checks for fs, which fails, and falls back to browser mode.

I'm also not sure it helps that wrangler is compiling my worker to a/var/folders/... dir 🤔 .

Hey @KrisBraun 👋 . Nice to see you here.

[xdivby0]

@xdivby0 it sounds like you may have it working?

I thought I did, but somewhere in the v2 preparations this broke and the approach I tried doesn't seem to work anymore. Currently driving blind so to say.

[BrandonNoad]

@xdivby0 Any ideas what may have changed to break your setup?

[xdivby0]

@xdivby0 Any ideas what may have changed to break your setup?

Unfortunately no, I am not really experienced in setting up sourcemap and error logging :/ I am still keeping an eye on this thread in case someone manages.

[jacob-ebey]

Thanks for the PR @fabiankaestner, but this has been addressed here: #7574