feat(swc): Add a new source maps option "linked"

[realtimetodie]

Adds a new source maps option "linked" to generate a source map file and append a sourceMappingURL comment to the end of the output, containing an URL to the generated source map file.

This is only implemented in @swc/cli

https://github.com/swc-project/cli/blob/933589ae907637afb7f0b0a43b7a24a7a920595e/src/swc/compile.ts#L31

Possible values for the --source-maps option

• false (default): Disables the source map generation.

• true: Generates a source map file, but without to append a sourceMappingURL comment to the end of the output.

• inline: Generates a source map file and appends a sourceMappingURL comment to the end of the output, containing the base64 encoded source map.

  //# sourceMappingURL=data:application/json;base64,<encoded_source_map_data>
  

New

• linked: Generates a source map file and appends a sourceMappingURL comment to the end of the output, containing an URL to the generated source map file.

  //# sourceMappingURL=<source_map_url>
  

Example

$ swc compile input.js --out-file=output.js --source-maps=linked

Output

output.js
output.js.map

Show the last line of the generated output.js file

$ tail -n=1 output.js
//# sourceMappingURL=output.js.map

Summary

• The source map path is now known to the compiler.

• Replaced the enum SourceMapsConfig::Str with the enum SourceMapsConfig::Inline and the enum SourceMapsConfig::Linked.

• Replaced the enum InputSourceMap::Str with the enum InputSourceMap::Inline.

• SourceMapsConfig::default() is now set to SourceMapsConfig::Bool(false) to match the compiler default (source map generation is disabled by default).

• Added the FromStr trait to SourceMapsConfig.

• Added the serde::Deserializer trait to SourceMapsConfig.

• Added the serde::Deserializer trait to InputSourceMap.

• Added tests

• Improved the error message when the SourceMapsConfig::from_str method is used

  Example

  $ swc compile input.js --out-file=output.js --source-maps=foo

  Error message

  invalid source maps value: got "foo" but expected either one of false, true, inline, linked
  

Related

• Introducing experimental native SWC CLI #3859
• SWCX (Native SWC binary) stablization plan #6479
• [Bug]: Sourcemap links are not generated aspect-build/rules_swc#146

[jridgewell]

This breaks with both Babel's and esbuild's definition of "both", which is to add an a //# sourceMappingURL=data:application/json;base64,… and have an external sourcemap file. Babel doesn't have an option like what's proposed here (probably because it doesn't' bundle), but esbuild calls this "linked"

[realtimetodie]

Thank you @jridgewell. This is a good idea, I like it.

I renamed the enum SourceMapsConfig::Both ("both") -> SourceMapsConfig::Linked ("linked") to not break with Babel's and esbuild's definition of "both" as suggested.

Example

$ swc compile input.js --out-file=output.js --source-maps=linked

[realtimetodie]

I updated the title and the initial comment.

[kdy1]

true: Generates a source map file, but without to append a sourceMappingURL comment to the end of the output

Is this true? I think this is a bug..

[kdy1]

Please use true for this instead

[kdy1]

Nevermind. I concluded linked is fine.

[realtimetodie]

@kdy1 I'm not sure why the bindings are in an isolated workspace. It complicates the development.

[kdy1]

I know, but there's a good reason

[alexeagle]

@realtimetodie do you think that build failure for the node binding is caused by this change?

error[E0061]: this function takes 12 arguments but 13 arguments were supplied
   --> binding_core_node/src/bundle.rs:127:51
    |
127 | ...                   let output = self.swc.print(
    |                                             ^^^^^
    |
note: associated function defined here
   --> /Users/runner/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/swc-0.242.2/src/lib.rs:438:12
    |
438 |     pub fn print<T>(
    |            ^^^^^
help: did you mean
    |
127 |                             let output = self.swc.print(&m, None, None, true, codegen_target, SourceMapsConfig::Bool(true), &Default::default(), None, minify, None, false, true)?;

[realtimetodie]

@alexeagle Yes, e4d8036 broke the build. Cargo pulls the swc_core crate from the remote crate registry now, which does not contain the new changes. Before that, the build passed. This is what I meant in #6817 (comment). I think @kdy1 will bump the version manually, once this is merged.

[alexeagle]

Ping @kdy1 - we're waiting for this one so we can release the Bazel swc integration to 1.0.0

[kdy1]

There's a merge conflict

[realtimetodie]

I resolved the merge conflict. The version of the swc_core crate is now set to 0.58.0.

[realtimetodie]

I had to delete the fixture test for the linked option due to the way the test runner works. It is not possible to construct a source map path from a .swcrc configuration alone, as the initial value is PathBuf::new().

[kdy1]

I was confused while reviewing for the first time, my bad.

Please see #6817 (comment)

[realtimetodie]

I think what you mean with "implicitly determined" is to generate the source maps path within the compiler from.the filename, instead of passing it down as an argument?

The reason why we can't "implicitly determine" the path to the source map file is because of the cli --source-map-target option.

Currently, the user can define a custom output path for the generated source maps file when using the SourceMapsConfig::Bool(true) enum. I simply adopted this feature for the new SourceMapsConfig::Linked enum too. This is the only reason why the source_maps_path is passed down to the compiler.

If you say that the cli --source-map-target option should be ignored let me know. I thought you saw this part in the code.

[kdy1]

I think what you mean with "implicitly determined" is to generate the source maps path within the compiler from.the filename, instead of passing it down as an argument?

Yep. I mean filename + .map, which is the previous logic.

But seems like it's my bad, sorry. I didn't know that --source-map-target exist. I'm not sure how did previous code work

[kdy1]

Ah, I got it. It simply returned the map object. But with linked it's impossible because linked requires writing the output path to the source file, right?

I'm sorry, I didn't think about this carefully. (I stopped thinking to work on stc while thinking about the source map path option)

[realtimetodie]

It is also my fault, I did not point this out in the initial comment when i listed the changes and features.

[kdy1]

Thank you, and sorry.

---

swc-bump:

• swc --breaking

[realtimetodie]

I deleted my previous comments. I was too emotional.

[kdy1]

Oh CI failed

[realtimetodie]

I need to revert 4b303a2.

[kdy1]

cc @kwonoj I think this can also be implemented by adding logic like https://github.com/swc-project/cli/blob/933589ae907637afb7f0b0a43b7a24a7a920595e/src/swc/compile.ts#L30-L33 in swc_cli.
Which way do you prefer?

[kdy1]

I thought more about this, and I think sourceMap=true should generate sourceMappingUrl, and linked seems a bit redundant. But this can't be done by the swc crate.

[realtimetodie]

I apologize for not writing my thoughts about the implementationin the initial post.

The main reason why I put the generation of the sourceMappingURL string into the swc crate is because I favor centralization. The code to generate the sourceMappingURL would be in one place, in the print function and would not be scattered around.

Secondly, I thought that third party consumers of the swc crate would greatly benefit from such a new feature SourceMapsConfig::Linked. Otherwise the feature would be hidden in the cli bindings crate, which is not meant to be consumed.

I understand the concerns about the new source_map_path variable. It seems unfortunate at first, but the path to the generated output is passed down using the output_path variable already.

swc/crates/swc/src/lib.rs

Lines 438 to 442 in 33dab56

	pub fn print<T>(
	&self,
	node: &T,
	source_file_name: Option<&str>,
	output_path: Option<PathBuf>,

I also want to emphasize on @jridgewell #6817 (comment). The introduction of a new "linked" option helps users, because the configuration and naming convention will be aligned with other tools.

[kdy1]

I thought more about this. I think an ideal solution is use SourceMapsConfig::Bool(true) for this behavior and patch @swc/cli not to add sourceMappingUrl.

The problem is that, @swc/cli

• generates sourceMappingUrl for SourceMapsConfig::Bool(true)
• generates two sourceMappingUrls for SourceMapsConfig::Linked

and swc_cli

• generates no sourceMappingUrl for SourceMapsConfig::Bool(true)
• generates one sourceMappingUrl for SourceMapsConfig::Linked

[realtimetodie]

@kdy1 Yes, the @swc/cli needs to be patched.

However, the Node.js cli can handle multiple inputs at once and concats the outputs. If we combine the SourceMapsConfig::Bool(true) option as suggested, this will create a regression because every generated output will contain an URL to a non-existent source map file

https://github.com/swc-project/cli/blob/7217f7ad6a68f5bc788094affd8c5530231911c5/src/swc/file.ts#L33

For this reason, we will always need an option to generate an output and a source map without a sourceMappingURL string.

Ideally, we could try to move this part from the Node.js cli into Rust. But this would require a lot of effort.
The new option SourceMapsConfig::Linked requires minimal changes to the Node.js cli and does not break existing behaviour.

[realtimetodie]

@kdy1 The @swc/cli can be patched in such a way, that users will not require to update their configuration (from true -> "linked") and the option "linked" would be invalid in Node.js.

[realtimetodie]

I linked to #1388 which is something I will be working on after this PR, as it is a bug in the Rust cli.

[kdy1]

I'm against adding an option which is invalid for @swc/cli