CI fails due to an unrelated error

Error: Couldn't find package "@babel/code-frame@^7.18.6" required by "@babel/template@^7.20.7" on the "npm" registry.

I pinned the version of the sourcemap crate to 6.2 globally.

After updating the sourcemap crate for swc_common, a fixture test fails. The string of base64 VLQs which contain the actual mappings of the generated source map are different.

crates/swc/tests/fixture/sourcemap/004

- "mappings": "AAACA,CAAAA,KAAKC,gBAAmB,GAAGD,KAAKC,gBAAmB,IAAI,EAAC,AAAD,EAAIC,IAAI,CAAC;IAC7D;QACI;KACH;IACD;QACU,MAAY,SAAUC,CAAuB,EAAEC,CAAmB,EAAEC,CAAmB,EAAE;YAC3F;YACA,IAAIC,IAAa,SAAoBC,CAAI,EAAE;gBACvC,IAAIC,IAAOD,EAAKC,IAAI;gBACpB,OAAsB,CAAA,GAAGC,EAA+CC,GAAE,AAAFA,EAAK,OAAO;oBAChFC,UAAUH,EAAKI,GAAG;gBACtB;YACJ;YACAP,EAAoBQ,CAAC,CAACT,IACDC,EAAoBS,CAAC,CAACV,GAAqB;gBACvCW,SAAS,WAAoB;oBAC9C,OAAqBA;gBACzB;gBACqBC,SAAS,WAAoB;oBAC9C,OAAqBV;gBACzB;YACJ,EAAA;YACqB,IAAIG,IAAiDJ,EAAoB,OAC1FU,IAAU,CAAA;QAE1B;QACc,MAAY,SAAUZ,CAAuB,EAAEc,CAAwB,EAAEZ,CAAmB,EAAE;YAC/Fa,CAAAA,OAAOC,QAAQ,GAAGD,OAAOC,QAAQ,IAAI,EAAC,AAAD,EAAIjB,IAAI,CAAC;gBAC3C;gBACA,WAAY;oBACR,OAAOG,EAAoB;gBAC/B;aACH;QAGb;IACI;IACS,SAAUA,CAAmB,EAAE;QAK3BA,EAAoBe,CAAC,CAAC,GAAG;YAC9B;YACA;YACA;SACH,EAAE,WAAY;YACX,OAPOf,EAAoBA,EAAoBgB,CAAC,GAOxB;QAC5B,IAESC,OAD0BjB,EAAoBe,CAAC,EAAA;IAGhE;CACC",
+ "mappings": "AAACA,CAAAA,KAAKC,gBAAmB,GAAGD,KAAKC,gBAAmB,IAAI,EAAC,AAAD,EAAIC,IAAI,CAAC;IAC7D;QACI;KACH;IACD;QACU,MAAY,SAAUC,CAAuB,EAAEC,CAAmB,EAAEC,CAAmB,EAAE;YAC3F;YACA,IAAIC,IAAa,SAAoBC,CAAI,EAAE;gBACvC,IAAIC,IAAOD,EAAKC,IAAI;gBACpB,OAAqB,AAAC,CAAA,GAAGC,EAA+CC,GAAE,AAAFA,EAAK,OAAO;oBAChFC,UAAUH,EAAKI,GAAG;gBACtB;YACJ;YACAP,EAAoBQ,CAAC,CAACT,IACDC,EAAoBS,CAAC,CAACV,GAAqB;gBACvCW,SAAS,WAAoB;oBAC9C,OAAqBA;gBACzB;gBACqBC,SAAS,WAAoB;oBAC9C,OAAqBV;gBACzB;YACJ,EAAA;YACqB,IAAIG,IAAiDJ,EAAoB,OAC1FU,IAAU,CAAA;QAE1B;QACc,MAAY,SAAUZ,CAAuB,EAAEc,CAAwB,EAAEZ,CAAmB,EAAE;YAC/Fa,CAAAA,OAAOC,QAAQ,GAAGD,OAAOC,QAAQ,IAAI,EAAC,AAAD,EAAIjB,IAAI,CAAC;gBAC3C;gBACA,WAAY;oBACR,OAAOG,EAAoB;gBAC/B;aACH;QAGb;IACI;IACS,SAAUA,CAAmB,EAAE;QAK3BA,EAAoBe,CAAC,CAAC,GAAG;YAC9B;YACA;YACA;SACH,EAAE,WAAY;YACX,OAPOf,EAAoBA,EAAoBgB,CAAC,GAOxB;QAC5B,IAESC,OAD0BjB,EAAoBe,CAAC,EAAA;IAGhE;CACC",

Should I revert the update of the sourcemap crate for swc_common or should I update the fixture test instead?

The snapshot itself should be updated in this case.

The diff is adding an additional AAAC, which in mappings terms translates to "add 1 to a column number". This is a valid behavior due to the fix that we had to apply here getsentry/rust-sourcemap#54 and which is based on discussion here getsentry/rust-sourcemap#53

I updated the fixture test. Thank you @kamilogorek.

Blocked by servo/string-cache#271

Some tests fail. GitHub actions is unable to run the git checkout step: HTTP status 429. I will retry tomorrow.

unable to access 'https://github.com/swc-project/swc/': The requested URL returned error: 429

The tests basically compare the generated source maps from the sourcemap crate with the generated source maps from the Node.js terser package

test result: FAILED. 1929 passed; 54 failed;

@kdy1 Is this a requirement to have parity with terser?

The reason why the tests fail seems to be related to getsentry/rust-sourcemap#53 as mentioned by @kamilogorek in #6973 (comment). Terser uses the Node.js @jridgewell/gen-mapping package to generate source maps. Was this issue in getsentry/rust-sourcemap#53 fixed in Node.js too?

Ping @jridgewell short summary: this PR updates the sourcemap crate to a version that includes your PR getsentry/rust-sourcemap#53. However, some tests are failing (see above).

It does not have to be same as terser. I just looked for a way to ensure that source maps are correct enough.
But not sure how to verify it without relying on some other tool or reviewing an enormous amount of source map changes.

@jridgewell Do you have any idea?

So the sourcemap itself hasn't changed:

[crates/swc_ecma_codegen/tests/sourcemap.rs:456] actual.tokens().collect::<Vec<_>>() = [
    <Token 017a45a1919f4006.js:0:0 (0:0)>,
    <Token 017a45a1919f4006.js:0:3 (0:2)>,
    <Token 017a45a1919f4006.js:0:10 (0:8)>,
    <Token 017a45a1919f4006.js:0:14 (0:12)>,
    <Token 017a45a1919f4006.js:0:16 (0:13)>,
    <Token 017a45a1919f4006.js:0:18 (0:14)>,
    <Token 017a45a1919f4006.js:0:27 (0:23)>,
    <Token 017a45a1919f4006.js:0:28 (0:24)>,
    <Token 017a45a1919f4006.js:0:29 (0:24)>,
]

Notice those last two tokens. They have the same generated location (0:24), but different source locations (0:28 vs 0:29). The bug we're seeing is the lookup token returning one or the other. Before, we'd see the 28, and now we're seeing the 29.

In getsentry/rust-sourcemap#54, we switched from a manual greatest-lower-bound binary search to one using Rust's binary_search_by_key. The big difference that I can see between the impls is the exact match condition: https://github.com/rust-lang/rust/blob/07c993eba8b76eae497e98433ae075b00f01be10/library/core/src/slice/mod.rs#L2529-L2532. Before, we'd continue searching even after finding an exact match, guaranteeing we'd get the lowest index exact-match. Now, we'll stop on any exact-match.

I think this is a bug in sourcemap, caused because I didn't really look up the binary search behavior when it was suggested we switch to the stdlib impl. For reference, my trace-mapping library guarantees the first/last item when tracing, Mozilla's v0.6.1 (the JS one) source-map does, but Mozilla's v0.7 (the Rust/WASM one) doesn't.

6.2.2 has been released, you can bump it here now

Ah god, I think I read it in the wrong direction. The test was failing because we received 28 where before it received 29 (the opposite of what I said in #6973 (comment)). From that, I assumed the old sourcemap code was returning the lowest match index, when in fact it seems like it was returning the highest match index. I assumed that because that's what the JS sourcemap packages do.

@kamilogorek: We could revert 6.2.2 and implement the highest-index behavior to match the old 6.2.0 behavior. Or we could align with the JS ecosystem, in which case SWC would need to skip these failing tests (or stop inserting multiple tokens for the same generated position).

I think it's better to align with js ecosystem, so we may inline source maps for those tests. But I think it should be done in a separate PR

@Swatinem what do you think?

I’m not sure I fully understand the problem at hand.

stop inserting multiple tokens for the same generated position

I believe this might be the solution. We shouldn’t even get to the situation where there is more than one token matching (and picking the lowest or higher match). What would that even mean in sourcemaps that you have more than one token for one minified source position?

@kdy I created a follow-up PR in #6989 and reverted the update for the swc component crates.

There were no changes in the main Cargo.lock file, the git history differed.