Handle source maps for nested assets where sourceMappingURL contains the subdirectory

[DimitriosLisenko]

We're using esbuild to compile our assets, and we have two separate application.css files - "application.css" and "javascript/application.css".

We've configured esbuild to have publicPath of "/assets", and we're seeing that the sourceMappingURL is correct for "application.css" but incorrect for "javascript/application.css".

Here's what the results of my testing was:

# no publicPath set in esbuild config
app/assets/builds/application.css => /*# sourceMappingURL=application.css.map */
app/assets/builds/javascript/application.css => /*# sourceMappingURL=application.css.map */
public/assets/application-ee89888f.css => /*# sourceMappingURL=/assets/application-2242cc06.css.map */
public/assets/javascript/application-7ca550ac.css => /*# sourceMappingURL=/assets/javascript/application-366197c5.css.map */

# publicPath: "assets" in esbuild config
WARNING LOGS: Removed sourceMappingURL comment for missing asset 'javascript/assets/javascript/application.css.map' from javascript/application.css

app/assets/builds/application.css => /*# sourceMappingURL=application.css.map */
app/assets/builds/javascript/application.css => /*# sourceMappingURL=assets/javascript/application.css.map */
public/assets/application-ee89888f.css => /*# sourceMappingURL=/assets/application-2242cc06.css.map */
public/assets/javascript/application-da8cc934.css => EMPTY

# publicPath: "/assets" in esbuild config
WARNING LOGS: Removed sourceMappingURL comment for missing asset 'javascript/javascript/application.css.map' from javascript/application.css

app/assets/builds/application.css => /*# sourceMappingURL=application.css.map */
app/assets/builds/javascript/application.css => /*# sourceMappingURL=/assets/javascript/application.css.map */
public/assets/application-ee89888f.css => /*# sourceMappingURL=/assets/application-2242cc06.css.map */
public/assets/javascript/application-57c47cf6.css => EMPTY

Note how it's basically joining the directories twice when publicPath is supplied (resulting in "javascript/javascript/application.css.map" which is invalid.

Given how in the case of no publicPath the sourceMappingURL just contains the basename of the asset map, it seems to me it's safe to get the basename from sourceMappingURL and then search for that file in the directory where the asset is - this will work as long as the asset and its map are in the same directory. I am not sure whether this is always the case, but it seems reasonable. Would be good for someone with more knowledge on this topic to weigh in!

Here is what it looks like post the changes in this branch:

# publicPath: "/assets" + File.basename change
app/assets/builds/application.css => /*# sourceMappingURL=application.css.map */
app/assets/builds/javascript/application.css => /*# sourceMappingURL=/assets/javascript/application.css.map */
public/assets/application-ee89888f.css => /*# sourceMappingURL=/assets/application-2242cc06.css.map */
public/assets/javascript/application-42990d6a.css => /*# sourceMappingURL=/assets/javascript/application-f62ecdcd.css.map */

This is based on the work done in #170.

Alternatively, perhaps what could be done is to get the common parts of the asset location and asset map location, and then join the remainder of the asset map path to the common part.

[theodorton]

[...] this will work as long as the asset and its map are in the same directory. I am not sure whether this is always the case, but it seems reasonable. Would be good for someone with more knowledge on this topic to weigh in!

I'm no expert on source maps, but given sourceMappingURL can point to absolute paths, I feel slightly uncomfortable merging with this assumption and potentially breaking more complex application scenarios.

I've pushed a change here, but had to make a change to one of the fixtures from #170 to get it to work. Would need a review from one of the other maintainers before this can be merged.

Could you pull the branch in your project and see if it works for your use case?

[theodorton]

@brenogazzola It looks to me like one of the test cases introduced in #170 assumes bad output from esbuild; a source map file contained in the nested directory is referred to living in the assets directory instead.

Before:

propshaft/test/fixtures/assets/mapped/nested/sourceMappingURL-already-prefixed-nested.js

Line 1 in 993c67d

var fun; //# sourceMappingURL=/assets/sourceMappingURL-already-prefixed-nested.js.map

After:

propshaft/test/fixtures/assets/mapped/nested/sourceMappingURL-already-prefixed-nested.js

Line 1 in c583d9c

var fun; //# sourceMappingURL=/assets/nested/sourceMappingURL-already-prefixed-nested.js.map

Any concerns making this change? The rest of the test suite passes, including the new case from this PR.

[DimitriosLisenko]

Confirming that it looks like this works for our use case! Also confirming that when I reviewed the tests, that particular test seemed like it was for invalid output from esbuild, but I'm not sure under what circumstances that would happen.

EDIT: Actually, I did stumble upon the invalid output case - it happened when outdir was to a subdirectory (app/assets/builds/monaco), and publicPath did not contain that subdirectory (/assets), which resulted in propshaft not being able to find the map since the directory structure doesn't match. Fixed that case by just changing publicPath to /assets/monaco though.

[theodorton]

@dhh any concerns merging this?

[dhh]

We don't use source maps, so I'd feel best if @brenogazzola or someone else who does can verify this.

[brenogazzola]

TIL: esbuild can handle CSS.

I’ll take a look when I have time to sit down and test in my own app (I also have multiple CSS files and some are nested), but sourcemap problems always take me a while because I use a pretty standard setup for them. But from a quick glance, if it’s the same problem as #170 the solution should be similar.