Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source maps generated in SASS output is unusable as served by trunk #899

Open
Kritzefitz opened this issue Oct 28, 2024 · 10 comments
Open
Labels
bug Something isn't working

Comments

@Kritzefitz
Copy link

When including SCSS or SASS assets in debug build, they are compiled using the --embed-source-maps option, which causes a source map to be genrated and included directly in the generated CSS asset. Unfortunately, this doesn't actually make it possible to actually use the source map in a browser to inspect the individual mapped sources of the CSS file. This is because the generated source file, doesn't contain the source code of the component files themselves, but merely references them via relative paths. But since the original source files will usually not be served along with the generated output, the browser won't be able to download those source files to display the correct source.

As an example, if I include a SCSS asset like this

<link rel="scss" href="styles/app.scss" />

and build it such that the HTML will be available at http://example.com/index.html, then the source map included in http://example.com/app-<hash>.css will reference the relative path styles/app.scss, causing the browser to request http://example.com/styles/app.css and treat the result as the expected source file. In the case of using the trunk development server, this won't return the expected source file, but will instead return the index.html, causing that to be displayed as the source file for the generated CSS

This leads to arguably worse results than having no source map at all, since e.g. Firefox will prefer to display the mapped source instead of the generated source if a source map is available, which in this case causes no actual CSS source to be displayed at all.

A relatively simple solution to this problem could be to call the sass compiler with --include-sources, which causes the source files to be included verbatim in the source map. Of course this also considerably increases the size of the source map, so it might be worth considering to remove the --embed-source-maps option altogether and instead copy the separate source map into the dist directory, so browser can fetch it independently of the CSS on demand.

@ctron
Copy link
Collaborator

ctron commented Oct 28, 2024

Hm, so it might make sense to keep the original path prefix (styles/ in this case)?

@ctron ctron added the bug Something isn't working label Oct 28, 2024
@Kritzefitz
Copy link
Author

I'm not sure that helps. In general, the generated CSS file and its source map (bundled or not) will end up in dist/ and that directory will be served by the webserver. But the source files referenced by the source map are not in dist/ and will thus not be available to the webserver. Unless we start copying all source files referenced by the source map into dist/, I don't think there's a way to get working source maps without --include-sources.

@ctron
Copy link
Collaborator

ctron commented Oct 28, 2024

Ok, I see. Thanks for explaining it. So maybe it makes sense to have --include-sources as a flag, and enable it by default for non-release builds?

@Kritzefitz
Copy link
Author

Yes, that seems like a good solution to me.

In addition it might make sense to stop using the --embed-source-map option and instead copy the *.css.map file do dist/ along with the *.css file. This should reduce the impact of the increased map size for clients/users that don't need the source map.

@ctron
Copy link
Collaborator

ctron commented Oct 28, 2024

Ok, I think I have a problem understanding the difference between --embed-source-map and --include-sources.

@Kritzefitz
Copy link
Author

Kritzefitz commented Oct 28, 2024

Ok, so on a fundamental level, sass turns a set of sass/scss file into one monolithic CSS file, i.e. when we originally had a.scss, b.scss and c.scss, sass turned them into one combined.css. This bundling of multiple SCSS files into one CSS file makes it more difficult to inspect the applied styles in most browser dev tools. To remedy this, there are source maps that document in a machine-readable format the relation between the whole generated CSS file ( combined.css) and the component source files that went into it (a.scss, b.scss, c.scss).

How exactly this mapping is communicated to the browser depends on various options, among them --embed-source-map and --include-source. If we don't pass any options, sass will generate a CSS (combined.css) and a separate source map (combined.css.map), where the latter will reference all the individual source files by path (i.e. a.scss, b.scss and c.scss). If a browser is asked to use combined.css as a style, it will also know to look for combined.css.map alongside it, to discover the appropriate debugging information. Inside the combined.css.map it will discover the references to a.scss, b.scss and c.scss and fetch each of those source files from the webserver separately, when debugging for them is requested. This assumes that all of those source files are actually served by the webserver (which they are usually not in our case).

--include-sources changes the way the original source files are included in the source map. Instead of the usual way of only referencing the original source files by path, the entire source code of the source files is copied into the source map itself, so combined.css.map now contains the sources for a.scss, b.scss and c.scss. This means that when the browser looks at the source map, it already has all the original sources it needs and there is no need to go and ask the webserver for the original source files (where they aren't available in our case).

--embed-source-map is more or less orthogonal to this. It doesn't affect the contents of the source map itself, it only changes the place where the generated source map is put into. As explained above, the default is to generate a separate file combined.css.map and put it next to the combined.css. This means that an interested browser needs to request the map separately from the CSS file. When --embed-source-map is used, there is no separate combined.css.map, instead the generated source map is embedded directly into the generated.css (you can see it as a huge cryptic comment at the end of the file). This means a browser doesn't need to request the file separately, as it is always transferred along with the CSS. This makes it slightly more convenient to deal with the source map, as it doesn't need to be separately copied along with the CSS, but it has the downside that every browser that wants to use the CSS also needs to download the entire source map contained in the same file. This downside is acceptable for debug scenarios, but it gets more pronounced as the size of the source map increases.

Note how we are talking about two different levels of indirection here:

CSS (combined.css) -> source map (combined.css.map) -> source files (a.scss, b.scss, c.scss)

Using --embed-source-map eliminates the first indirection between CSS and the source map, while --include-sources eliminates the second level between the source map and the source files.

So the two options do similar, but ultimately orthogonal things. I think they are both relevant here, because --include-sources actually fixes the bug we have, but would considerably increase the size of the generated CSS file. To neutralize this increased CSS size, I propose to drop the use of --embed-source-map so the source map ends up in its separate combined.css.map where the increased size only matters for clients that actually need that debugging information.

@ctron
Copy link
Collaborator

ctron commented Oct 29, 2024

Thanks for the thorough explanation! I believe I have a much better understanding of this now.

So a quick fix would be to use both flags for debug mode. And none for release mode.

A better approach might be to use --include-sources only, for both debug and release mode. But that would also mean that the file would need to be renamed to foo-<digest of foo.css>.css.map, right? Making this a bit more tricky. But would allow to debug CSS issues with a release build too?

@Kritzefitz
Copy link
Author

Yes exactly, that matches my understanding.

@Kritzefitz
Copy link
Author

Ah, there's some subtlety here, that I didn't account for. Even with a separate foo.css.map (i.e. without --embed-source-map) the CSS file needs to declare where its source map is found, e.g. with a comment like this:

/*# sourceMappingURL=foo.css.map */

This comment is usually generated by the sass compiler, so the included path doesn't include the digest generated by trunk. So if we drop --embed-source-map we'd have to copy foo.css.map without the digest. I've verified in tests that this works, but I think it might interfere with the caching of many production setups, which would then apply aggressive caching to foo.css.map under the assumption that it contains a tag to invalidate the cache on changes.

@Kritzefitz
Copy link
Author

I just noticed that I messed up the option name for the entire conversation. The option to include source files in the generated source map, previously referred to as --include-sources, is actually called --embed-sources, making the two options even more difficult to differentiate. See here: https://sass-lang.com/documentation/cli/dart-sass/#embed-sources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants