Fix broken sourcemaps when using `esbuild --public-path`

[richardkmiller]

trafficstars

When the --public-path option was added to esbuild it broke Sprockets's ability to resolve sourcemaps, even though they're still present.

Esbuild's sourcemap output with --public-path=assets: //# sourceMappingURL=assets/application.js.map

Esbuild's sourcemap output without --public-path: //# sourceMappingURL=application.js.map

It's desirable to keep --public-path=assets for properly resolving static assets in JavaScript files, but the leading directory in sourceMappingURL is undesirable, or at least it does not work with Sprockets.

This PR allows sourceMappingURL= to contain a leading, optional directory which is ignored as the path is resolved.

This is an alternative to #501. (Sorry, I didn't see it until after I found this solution.)

[capripot]

Are we still planning on merging this? Should we fix errors from tests?

[guillaumebriday-pa]

@dhh could you take a look please?

Thanks

[dhh]

Thanks for looking into this. Needs testing to verify behavior.

[joevandyk]

I am also running into this as well with esbuild, I have to remove the --public-path option from esbuild to get sourcemaps to work, otherwise sprockets-rails seems to remove the sourcemap comment that esbuild creates.

[joevandyk]

would the propshaft fix at https://github.com/rails/propshaft/pull/170 be applicable here?

[nicklozon]

@richardkmiller would you like me to write test cases and maybe we can get this merged?

[richardkmiller]

@nicklozon Yes, that'd be great. Thanks for asking!

[nicklozon]

@richardkmiller I wrote a test and opened a PR against your repo: https://github.com/richardkmiller/sprockets-rails/pull/1

I think the change makes sense and works - sprockets-rails assumes the map file will exists in the same directory as the source file so it stands to reason that any prefixed path should be removed from the sourceMappingUrl.

[richardkmiller]

@nicklozon Thank you for these tests! I've merged them just now.

[jroes]

Would love to see this merged! Can confirm this is resolving some issues on my end.

[zacheryph]

Any ideas on when this would possibly get merged [and released] ?

[qmenoret]

Clearly not a great option but I managed to hack something together with a esbuild plugin that I use only in dev mode for now:

const pathSourceMapsPlugin = {
  name: "pathSourceMaps",
  setup({ onStart, onEnd, initialOptions }) {
    initialOptions.write = false

    onEnd(async (result) => {
      const tasks = []
      for (let { path, contents, text } of result.outputFiles) {
        if (path.match(/application.js/)) {
          const newValue = text.replace(
            /\/\/# sourceMappingURL=\/assets\/application.js.map/g,
            "//# sourceMappingURL=application.js.map",
          )
          contents = new TextEncoder().encode(newValue)
        }
        tasks.push(promises.writeFile(path, contents))
      }

      await Promise.all(tasks)
    })
  },
}

It's basically hardcoding the sourcemap path, but at least I get the sourcemaps 😅

[dhh]

cc @rafaelfranca, do you or anyone on the team have some bandwidth to verify this? Sprockets is a bit of an unruly beast at the moment, and the criticality is high. We don't use Sprockets in active development any more (moved to Propshaft), so I'd feel better if someone who does signs off on this.

[fredrivett]

Appreciate the work everyone is doing on rails, just wanted to chime in with another vote on this one. Have spent the last couple days trying to work out why I couldn't upload sourcemaps generated with esbuild and then processed by the rails asset pipeline to datadog, and it looks like this issue is at least a part of the problem.