fix(common): Fix handling of input source maps #6561

kdy1 · 2022-12-01T05:04:48Z

Description:

Related issue:

kdy1

swc-bump:

swc_common

crates/swc/tests/source_map.rs

kdy1 · 2022-12-01T06:51:48Z

@jridgewell Can you take a look? This is not related to the minifier, but I need your help

jridgewell · 2022-12-01T10:11:25Z

crates/swc/tests/fixture/sourcemap/004/output/index.map

@@ -1,5 +1,5 @@
 {
-  "mappings": "AEACA,CAAAA,KAAK,gBAAmB,GAAGA,KAAK,gBAAmB,IAAI,EAAE,AAAD,EAAGC,IAAI,CAAC;IAC7D;QAAC;KAAI;IACL;QACU,MAAY,SACdC,CAAuB,EACvBC,CAAmB,EACnBC,CAAmB,EACrB;YACE;gBFNXC,IAAA,SAAAC,CAAA,EAAA;gBAAA,IAAAC,IAAAD,EAAAC,IAAA;gBAAA,OAAA,CAAA,GCDDC,EAAAC,GAAA,EAAA,OAAA;oBACAC,UAAAH,EAAAI,GAAA;gBACA;YACA;YAKYP,EAAoBQ,CAAC,CAACT,IACDC,EAAoBS,CAAC,CAACV,GAAqB;gBACvCW,SAAS,WAAY;oBACtC,OAAqBA;gBDZjB;gBAAqBC,SAAA,WAAA;oBAC3C,OAAOV;gBAAM;YACd;YAAA,IAAAG,IAAAJ,EAAA,OAAAU,IAAA,CAAA;QCKM;QAKP,MAAA,SAAAZ,CAAA,EAAAc,CAAA,EAAAZ,CAAA,EAAA;YAAAa,CAAAA,OAAAC,QAAA,GAAAD,OAAAC,QAAA,IAAA,EAAA,EAAAjB,IAAA,CAAA;gBAAA;gBAAA,WAAA;oBAAA,OAAAG,EAAA;gBAAA;aAAA;QAAA;IAAA;IAAA,SAAAA,CAAA,EAAA;QAAAA,EAAAe,CAAA,CAAA,GAAA;YAAA;YAAA;YAAA;SAAA,EAAA,WAAA;YAAA,OAAAf,EAAAA,EAAAgB,CAAA,GAAA;QAAA,IAAAC,OAAAjB,EAAAe,CAAA;IAAA;CAAA",
+  "mappings": ";;;KACS",


This worries me, but it seems to be the input sourcemap is really bad:

input

output (old)

output (new)

crates/swc_common/src/source_map.rs

jridgewell · 2022-12-01T10:21:54Z

crates/swc_common/src/source_map.rs

@@ -1281,12 +1281,17 @@ impl SourceMap {
            let mut col = max(chpos, linechpos) - min(chpos, linechpos);

            if let Some(orig) = &orig {
-                if let Some(token) = orig.lookup_token(line, col) {


There's actually (what I consider to be) a bug in lookup_token, where it'll return an a mapping span on a previous line for spans that:

extend beyond the last mapped line

lines that don't have a mapping

unmapped columns on lines with a mapping

I tried to get them to fix it in getsentry/rust-sourcemap#53, but they rejected it. This breaks with Mozilla's source map behavior, because lines endings are supposed to split the mapping from any previous one.

Anyways, the "fix" is to do:

orig.lookup_token(line, col).filter(|t| t.get_dst_line() == line)

Is this the bug you're trying to fix with the find?

Yes, exactly. Thank you!

I'm not sure about the reason, but it didn't work.

Ohh, we probably have an off-by-one. The visualized link you posted is definitely doing the lookup on the wrong line. The sourcemap crate uses 0-based lines, based on the readme example.

kdy1 · 2022-12-02T03:48:51Z

crates/swc/tests/fixture/sourcemap/004/output/index.map

@@ -1,38 +1,33 @@
 {
-  "mappings": "AEACA,CAAAA,KAAK,gBAAmB,GAAGA,KAAK,gBAAmB,IAAI,EAAE,AAAD,EAAGC,IAAI,CAAC;IAC7D;QAAC;KAAI;IACL;QACU,MAAY,SACdC,CAAuB,EACvBC,CAAmB,EACnBC,CAAmB,EACrB;YACE;gBFNXC,IAAA,SAAAC,CAAA,EAAA;gBAAA,IAAAC,IAAAD,EAAAC,IAAA;gBAAA,OAAA,CAAA,GCDDC,EAAAC,GAAA,EAAA,OAAA;oBACAC,UAAAH,EAAAI,GAAA;gBACA;YACA;YAKYP,EAAoBQ,CAAC,CAACT,IACDC,EAAoBS,CAAC,CAACV,GAAqB;gBACvCW,SAAS,WAAY;oBACtC,OAAqBA;gBDZjB;gBAAqBC,SAAA,WAAA;oBAC3C,OAAOV;gBAAM;YACd;YAAA,IAAAG,IAAAJ,EAAA,OAAAU,IAAA,CAAA;QCKM;QAKP,MAAA,SAAAZ,CAAA,EAAAc,CAAA,EAAAZ,CAAA,EAAA;YAAAa,CAAAA,OAAAC,QAAA,GAAAD,OAAAC,QAAA,IAAA,EAAA,EAAAjB,IAAA,CAAA;gBAAA;gBAAA,WAAA;oBAAA,OAAAG,EAAA;gBAAA;aAAA;QAAA;IAAA;IAAA,SAAAA,CAAA,EAAA;QAAAA,EAAAe,CAAA,CAAA,GAAA;YAAA;YAAA;YAAA;SAAA,EAAA,WAAA;YAAA,OAAAf,EAAAA,EAAAgB,CAAA,GAAA;QAAA,IAAAC,OAAAjB,EAAAe,CAAA;IAAA;CAAA",
+  "mappings": "MACIC,gBAAA,GAAAD,KAAAC,gBAAA,IAAA,EAAA,EAAAC,IAAA,CAAA;;QAEC;KACD;;cAEQ,SAAAC,CAAA,EAAAC,CAAA,EAAAC,CAAA,EAAA;YACA;gBACIC,IAAI,SAAYH,CAAI,EAAA;gBACpB,IAAAC,IAAsBD,EAAAI,IAAkD;uBACpE,CAAA,GAAAC,EAAAC,GAAe,AAAL,EAAK,OAAG;oBACtBC,UAAAN,EAAAO,GAAA;gBACJ;YACA;gBAEyBP,IAAAC,EAAAO,CAAA,CAAAR,GAAS;yBAC1B,WAAqB;oBACzB,OAAAU;gBACqB;yBACjB,WAAqB;oBACzB,OAAAR;gBACJ;YACqB,EAAA;YAGjC,IAAAE,IAAAH,EAAA,OAAAS,IAAA,CAAA;QACc;cACD,SAAOX,CAAA,EAAAC,CAAA,EAAQC,CAAA,EAAG;oBACfY,QAAA,GAAAD,OAAAC,QAAA,IAAA,EAAA,EAAAf,IAAA,CAAA;gBACA;2BACW;oBACX,OAAAG,EAAA;gBACH;aAGb;QACI;IACS;aAK0BF,CAAA,EAAG;YAC9B,GAAA;YACA;YACA;YACD;sBAN4B;YAUtB,OAD0BA,EAAoBA,EAACgB,CAAA,GAAA;QAGhE,IAAAC,OAAAjB,EAAAe,CAAA,EAAA;IACC",


I'll fix this with another PR
(Seems like a source map issue related to codegen of multiline comments.)

lookup_token with filter was the cause.

Visualized

This reverts commit 87115e7.

Fixes a few things: - Propagates the input's sourcesContent item - Propagates the input's original name, if found (else, uses our name) - Uses much faster lookup_token (instead of a full traversal, that would be sooo slow)

jridgewell

Updated visualization

jridgewell · 2022-12-02T04:32:36Z

crates/swc_common/src/source_map.rs

@@ -1281,12 +1281,17 @@ impl SourceMap {
            let mut col = max(chpos, linechpos) - min(chpos, linechpos);

            if let Some(orig) = &orig {
-                if let Some(token) = orig.lookup_token(line, col) {


Ohh, we probably have an off-by-one. The visualized link you posted is definitely doing the lookup on the wrong line. The sourcemap crate uses 0-based lines, based on the readme example.

jridgewell · 2022-12-02T04:33:44Z

crates/swc_common/src/source_map.rs

+                if let Some(token) = orig
+                    .tokens()
+                    .find(|token| token.get_src_line() == line - 1 && token.get_src_col() == col)
+                {
                    line = token.get_src_line() + 1;
                    col = token.get_src_col();


I notice we're not pulling the name from the found token, which means we'll always the name found in SWC's input. But, the name could have already been changed by whatever preceeded SWC.

crates/swc/tests/fixture/sourcemap/004/output/index.map

kdy1 added 4 commits December 1, 2022 13:53

Add .swcrc

71d3cfc

sourcemap

1efa944

swcrc

81537e8

Add a test

c729d9c

kdy1 added this to the Planned milestone Dec 1, 2022

kdy1 self-assigned this Dec 1, 2022

kdy1 changed the title ~~fix(es/transforms): Fix sourcemap~~ fix(es): Fix input source map option Dec 1, 2022

Update test refs

6ecbec9

kdy1 changed the title ~~fix(es): Fix input source map option~~ fix(common): Fix input source map option Dec 1, 2022

kdy1 changed the title ~~fix(common): Fix input source map option~~ fix(common): Fix handling of input source maps Dec 1, 2022

kdy1 added 9 commits December 1, 2022 14:28

Not sure

ff145a0

Add a test

767c047

Fix base64 decode

4d1873d

Use Url

ea276d9

Dep

2ad8cc1

Fix import

7ff08d4

fix

c638250

Improve a bit

d724867

Fix

2b636f2

kdy1 commented Dec 1, 2022

View reviewed changes

crates/swc/tests/source_map.rs Show resolved Hide resolved

kdy1 requested a review from jridgewell December 1, 2022 06:51

kdy1 marked this pull request as ready for review December 1, 2022 06:53

kodiakhq bot previously approved these changes Dec 1, 2022

View reviewed changes

Update test refs

f65e03b

kdy1 dismissed kodiakhq[bot]’s stale review via f65e03b December 1, 2022 07:46

kodiakhq bot previously approved these changes Dec 1, 2022

View reviewed changes

jridgewell suggested changes Dec 1, 2022

View reviewed changes

Use lookup_token with filter

87115e7

kdy1 added 2 commits December 2, 2022 12:41

Fix input

811d3a3

Update test ref

e0ab2ad

kdy1 dismissed kodiakhq[bot]’s stale review via e0ab2ad December 2, 2022 03:45

kodiakhq bot previously approved these changes Dec 2, 2022

View reviewed changes

kdy1 commented Dec 2, 2022

View reviewed changes

kdy1 requested a review from jridgewell December 2, 2022 03:49

Revert "Use lookup_token with filter"

5d5cdff

This reverts commit 87115e7.

kdy1 dismissed kodiakhq[bot]’s stale review via 5d5cdff December 2, 2022 04:05

kodiakhq bot previously approved these changes Dec 2, 2022

View reviewed changes

Update test ref

8ecee8d

kdy1 dismissed kodiakhq[bot]’s stale review via 8ecee8d December 2, 2022 04:06

kodiakhq bot previously approved these changes Dec 2, 2022

View reviewed changes

Update sourcemap tracing logic

9939ca0

Fixes a few things: - Propagates the input's sourcesContent item - Propagates the input's original name, if found (else, uses our name) - Uses much faster lookup_token (instead of a full traversal, that would be sooo slow)

jridgewell dismissed kodiakhq[bot]’s stale review via 9939ca0 December 2, 2022 05:42

kodiakhq bot previously approved these changes Dec 2, 2022

View reviewed changes

jridgewell previously approved these changes Dec 2, 2022

View reviewed changes

clippy

96f07c5

kdy1 enabled auto-merge (squash) December 2, 2022 07:10

kdy1 dismissed stale reviews from jridgewell and kodiakhq[bot] via 96f07c5 December 2, 2022 07:10

kodiakhq bot approved these changes Dec 2, 2022

View reviewed changes

Merge branch 'main' into srcmap-fast

ddaabee

kdy1 merged commit 4af52c7 into swc-project:main Dec 2, 2022

kdy1 deleted the srcmap-fast branch December 2, 2022 07:58

This was referenced Dec 2, 2022

Next + SWC appear to generate incorrect sourcemaps vercel/next.js#39878

Closed

Small cleanup to the source_map module #6570

Merged

kdy1 mentioned this pull request Dec 4, 2022

chore(next-swc): Update swc_core to v0.45.3 vercel/next.js#43449

Closed

kdy1 modified the milestones: Planned, v1.3.22 Dec 9, 2022

swc-project locked as resolved and limited conversation to collaborators Jan 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(common): Fix handling of input source maps #6561

fix(common): Fix handling of input source maps #6561

kdy1 commented Dec 1, 2022 •

edited

Loading

kdy1 left a comment •

edited

Loading

kdy1 commented Dec 1, 2022

jridgewell Dec 1, 2022

jridgewell Dec 1, 2022

kdy1 Dec 2, 2022

kdy1 Dec 2, 2022

jridgewell Dec 2, 2022

kdy1 Dec 2, 2022

kdy1 Dec 2, 2022

kdy1 Dec 2, 2022

jridgewell left a comment

jridgewell Dec 2, 2022

jridgewell Dec 2, 2022

fix(common): Fix handling of input source maps #6561

fix(common): Fix handling of input source maps #6561

Conversation

kdy1 commented Dec 1, 2022 • edited Loading

kdy1 left a comment • edited Loading

Choose a reason for hiding this comment

kdy1 commented Dec 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jridgewell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdy1 commented Dec 1, 2022 •

edited

Loading

kdy1 left a comment •

edited

Loading