fix(transformer): remove span of define value

[hi-ogawa]

• Closes define generates incorrect sourcemap #7797

Removing span seems to work, but I'm not sure traversing all the time is okay.

[graphite-app]

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

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

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[codspeed-hq]

CodSpeed Performance Report

Merging #7811 will not alter performance

_{Comparing hi-ogawa:fix-define-span (6149889) with main (65b109a)}

Summary

✅ 29 untouched benchmarks

[overlookmotel]

I don't think this will generalize well. As I understand it, the replacement expression can be anything, and this will not remove all the spans from e.g. (() => { return 123; })().

To solve this fully, we either need:

1. Option to parser to produce empty spans for all nodes.
2. VisitMut implementation which removes span from all nodes. This could be generated using ast_tools codegen.

The 1st option would be faster for this particular use case, but we'd need to make sure it doesn't produce a noticeable slowdown for parsing in "standard mode". We could make it a generic param ParserImpl<const EMPTY_SPANS: bool> but that'd produce a large binary size increase.

So probably 2nd option is better. The AST for replacement expression is generally small, so traversing it will not be too costly.

[hi-ogawa]

As I understand it, the replacement expression can be anything, and this will not remove all the spans from e.g. (() => { return 123; })().

I was wondering about that too. Esbuild doesn't allow arbitrary expression (probably it supports only json value, identifier, member expression https://esbuild.github.io/api/#define), but oxc can do different.

For the usage I'm aware of, I thought this could be a reasonable fix to start with, but if better fix is desired, I would be happy to try ast_tools approach as I'm interested in it (and I'm scared to touch parser code 😅). Please let me know!

[overlookmotel]

#7816 adds a visit_span method to Visit and VisitMut. This would make implementing RemoveSpans trivial:

impl<'a> VisitMut<'a> for RemoveSpans {
  fn visit_span(&mut self, span: &mut Span) {
    *span = SPAN;
  }
}

Only problem is that #7816 may cause a perf regression. I hope it won't as visit_span is a no-op by default, but let's see what benchmarks say.

Note: I suggest calling the visitor RemoveSpans (plural), not RemoveSpan, because this will remove all spans, not just one.

[overlookmotel]

Looks like #7816 has no perf impact. If Boshen is happy with it, it will solve the problem in this PR.

[overlookmotel]

Could you please add a test where replacement contains statements e.g. (() => { return 123; })()?