Enhance sourceMapIncludeSources option by ntkme · Pull Request #2713 · sass/dart-sass

ntkme · 2025-12-26T20:05:38Z

Fixes sourceMapUrl fallback in ImporterResult creates bloated source map output with source contents as source urls #2712

sass/sass#4170
sass/sass-spec#2097
sass/embedded-host-node#411

nex3

Just a preliminary review so far.

nex3 · 2026-02-12T00:06:12Z

lib/src/util/source_map.dart

+
+import '../source_map_include_sources.dart';
+
+Map<String, dynamic> sourceMapToJson(SingleMapping sourceMap,


Please document this.

nex3 · 2026-02-12T00:07:21Z

lib/src/source_map_include_sources.dart

+
+  @Deprecated('Use always instead.')
+  true_,
+
+  @Deprecated('Use never instead.')
+  false_,


There's no need to include these in the Dart enum. It's not actually providing any more backwards-compatibility.

I added these to throw deprecation warnings… It turns out the logger for throwing deprecation warnings isn’t accessible at JS interop code so I had to pass these value into Dart code and throw it there.

nex3 · 2026-02-12T00:09:07Z

lib/src/source_map_include_sources.dart

+///
+/// {@category Compile}
+enum SourceMapIncludeSources {
+  /// Let compiler decide whether to include each source content.


We can be more specific about this: source contents are included only for sources for which ImporterResult.sourceMapUrl isn't set.

nex3 · 2026-02-12T00:14:28Z

lib/src/util/source_map.dart

+          sourceMapIncludeSources == SourceMapIncludeSources.always ||
+              (sourceMapIncludeSources == SourceMapIncludeSources.auto &&
+                  sourceMap.files.any((file) => file != null)));


I don't think this logic is right, for several reasons. First, Sass only deals in FileSpans—every span will have a SourceFile, because we know the whole file it was parsed from. None of these should be null, even if they don't have a defined source map URL. Second, this flag is all-or-nothing: it'll either include the sourcesContent for every file in the map, or for none of them. I think we'll need to submit an upstream change to make it possible to select whether to include the contents on a file-by-file basis.

This flag is to change whether the output source map will have sourceContent key or not. E.g. For never we can omit the whole sourceContent key. For autowe can omit the key if every sourceContent is null.

Right, but checking whether files are null isn't the right way to do that... every SourceLocation we pass in will be a FileLocation, and so have a non-null file. In addition, we need to filter out the contents of files whose URLs we know in auto mode.

every SourceLocation we pass in will be a FileLocation

That's exactly what I was saying, that the Dart API's default was "always".

For "auto" and "never" to actually work for Dart API, they are conditionally set to null here in post processing: https://github.com/ntkme/dart-sass/blob/0cceb8aa1de82c58123dd93a11106cff62c13dbd/lib/src/async_compile.dart#L242-L251

From Dart's source_maps lib:
https://pub.dev/documentation/source_maps/latest/source_maps.parser/SingleMapping/files.html

Files whose contents aren't available are null.

nex3 · 2026-02-12T00:16:00Z

lib/src/importer/result.dart

-  Uri get sourceMapUrl =>
-      _sourceMapUrl ?? Uri.dataFromString(contents, encoding: utf8);
-  final Uri? _sourceMapUrl;
+  /// An absolute URL indicating the resolved location of the imported stylesheet.


It's probably worth also describing how this interacts with the sourceMapIncludeSources option.

lib/src/importer/result.dart

nex3 · 2026-02-12T00:19:53Z

lib/src/async_compile.dart

+  SourceMapIncludeSources sourceMapIncludeSources =
+      SourceMapIncludeSources.always,


Shouldn't this default to auto?

Also, in Dart it's generally a good practice to avoid default parameters for anything other than booleans, because it breaks the ability for callers to pass in null for "do the default behavior". Better to make this nullable and handle the fallback in the method body.

The previous behavior of Dart API without this option is effectively “always”, the previous “auto” behavior is done via post processing in Dart-JS code only.

I'm okay with changing the default here for Dart Sass callers, since the previous default didn't make a lot of sense anyway. Either way, this should by nullable and handle the default in the body.

nex3 · 2026-02-12T00:21:25Z

lib/src/async_compile.dart

+      logger?.warnForDeprecation(
+        Deprecation.sourceMapIncludeSourcesBoolean,
+        'Passing boolean value for Options.sourceMapIncludeSources is '
+        'deprecated and will be removed in Dart Sass 2.0.0.\n\n'
+        'More info: https://sass-lang.com/d/source-map-include-sources-boolean',
+      );


This should be handled at the JS level.

The logger object isn’t available at JS level. Also throwing it here allow us to only need to throw in a single code path, instead of need to duplicate it in multiple code paths.

In fact, if you just look at a few lines above, the legacy JS API warning is done here, instead of in the JS API code, I think for exactly same reason...

I think that's probably something we'll need to fix sooner or later anyway as APIs evolve over time. The DeprecationProcessingLogger initialization isn't that complex; we should be able to copy it into the JS code and then check if the logger that's passed in to compile*Async is already deprecation-processing and avoid rewrapping if so.

Either way, we shouldn't expose immediately-deprecated enum fields in a new Dart API. If we had to handle that, we should either make this parameter Object or have two separate enums.

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 99f61c2 to a4cac51 Compare December 26, 2025 20:07

ntkme mentioned this pull request Dec 26, 2025

Add support for source_map option in dart-sass filter nanoc/nanoc#1775

Merged

1 task

ntkme marked this pull request as draft December 26, 2025 22:18

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 76ad1db to 8b82801 Compare December 27, 2025 07:07

ntkme mentioned this pull request Dec 27, 2025

Enhance sourceMapIncludeSources option sass/embedded-host-node#411

Open

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 8b82801 to a3e2139 Compare December 27, 2025 08:10

ntkme marked this pull request as ready for review December 27, 2025 08:17

ntkme changed the title ~~Use the canonical URL instead of a data: URL for sourceMapUrl fallback~~ Enhance sourceMapIncludeSources option Dec 27, 2025

ntkme mentioned this pull request Dec 27, 2025

Enhance sourceMapIncludeSources option sass/sass-spec#2097

Open

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch 13 times, most recently from 8fe2160 to 44acc05 Compare December 28, 2025 04:38

ntkme marked this pull request as draft December 28, 2025 19:04

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch 4 times, most recently from c8be35d to 4564d85 Compare December 28, 2025 20:22

ntkme marked this pull request as ready for review December 28, 2025 20:27

ntkme mentioned this pull request Dec 28, 2025

sourceMapUrl fallback in ImporterResult creates bloated source map output with source contents as source urls #2712

Open

9 tasks

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 4564d85 to 331db67 Compare December 28, 2025 21:59

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 331db67 to 7b78a30 Compare December 28, 2025 22:09

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch 2 times, most recently from d39b1dc to 7f1e685 Compare January 6, 2026 01:14

Goodwine added the enhancement label Jan 12, 2026

Goodwine requested a review from nex3 January 12, 2026 21:51

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from 7f1e685 to 6555562 Compare January 21, 2026 23:48

ntkme added 2 commits January 28, 2026 18:54

Use the canonical URL instead of a data: URL for sourceMapUrl fallback

116174c

Enhance sourceMapIncludeSources option

1f21b67

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch 5 times, most recently from 4ff78a6 to be48ba7 Compare January 29, 2026 04:00

nex3 requested changes Feb 12, 2026

View reviewed changes

Emit deprecation

c4a7a9a

ntkme force-pushed the canonical-url-as-source-map-url-fallback branch from be48ba7 to c4a7a9a Compare February 12, 2026 03:45

Add sourceMapUrlOrNull

0cceb8a


		import '../source_map_include_sources.dart';

		Map<String, dynamic> sourceMapToJson(SingleMapping sourceMap,

		SourceMapIncludeSources sourceMapIncludeSources =
		SourceMapIncludeSources.always,

Conversation

ntkme commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nex3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ntkme Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ntkme commented Dec 26, 2025 •

edited

Loading

ntkme Feb 13, 2026 •

edited

Loading