fix(transformer): RegExp transform only set span on final expression

[overlookmotel]

No description provided.

[overlookmotel]

• fix(transformer): RegExp transform only set span on final expression #5508 : 2 dependent PRs (#5511 , #5527 ) 👈
• refactor(transformer): RegExp transform unbox early #5504
• fix(transformer): RegExp transform handle Term::Quantifier #5501
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @overlookmotel and the rest of your teammates on Graphite

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[codspeed-hq]

CodSpeed Performance Report

Merging #5508 will not alter performance

_{Comparing 09-05-fix_transformer_regexp_transform_only_set_span_on_final_expression (9b7ecc7) with main (fad0a05)}

Summary

✅ 29 untouched benchmarks

[overlookmotel]

I think this is correct. We're transforming the whole RegExpLiteral into a NewExpression.

In: /abc/u
Out: new RegExp("abc", "u")

So I think:

1. The entire new RegExp(...) expression should have the span of the original RegExpLiteral
2. All the component parts (RegExp, "abc", "u") should have anonymous spans.

If I'm wrong, and we do want the component parts to have real spans, I guess they should reference the corresponding sections of the original regexp, not it's entirety.

 ___ span 1
/abc/u
     ^ span 2

            ___ span 1
new RegExp("abc", "u")
                   ^ span 2

But I'm not that knowledgeable on source maps. @Dunqing @Boshen can you please advise?

[overlookmotel]

It's unclear what Babel is doing concerning spans. This doesn't set spans on inner components, but it looks like it doesn't set span on the whole expression either!

https://github.com/babel/babel/blob/main/packages/babel-plugin-transform-sticky-regex/src/index.ts

[graphite-app]

Merge activity

• Sep 6, 7:52 AM EDT: Dunqing added this pull request to the Graphite merge queue.
• Sep 6, 8:09 AM EDT: Dunqing merged this pull request with the Graphite merge queue.

[Dunqing]

I'm not that knowledgeable on source maps too. I checked esbuild does. Looks like it does well. You can see the source-map-visualization

[Dunqing]

So far, I haven't checked to see if I've set the span in the transformer correctly. 😅

[Dunqing]

Once the merge label is added, no one can stop Graphite from merging PR🥲

[overlookmotel]

You can stop it by removing the merge label if you do that before it actually starts the merge process. But once the wheels are in motion, the train has no brakes and will continue rumbling down the track no matter what you do. Choo choo!