fix(transformer): Correctly trim JSX

[magic-akari]

• Closes: Transform: Tab \t in JSX should be trimmed #6638

[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 #6639 will not alter performance

_{Comparing fix/issue-6638 (1d3d256) with main (9281234)}

Summary

✅ 30 untouched benchmarks

[overlookmotel]

I wonder if we should be doing this in parser not transformer (see #6638 (comment)). But, even if that's the case, it'd be good to get this merged as the output at present is terrible!

One question:

It looks like is_white_space_single_line should be equivalent to:

fn is_white_space_single_line(c: char) -> bool {
    matches!(c, SP | TAB) || is_irregular_whitespace(c)
}

That'd be preferable, as it's easier to understand (though the extra constants this PR adds are really helpful - we should use them in is_irregular_whitespace).

But... why does is_white_space_single_line return true for ZWSP (\u{200b}), and is_irregular_whitespace returns false? Is is_irregular_whitespace wrong, or is is_white_space_single_line wrong? The discrepancy is:

• is_irregular_whitespace matches '\u{2000}'..='\u{200a}'
• is_white_space_single_line matches EN_QUAD..=ZWSP, which is equivalent to '\u{2000}'..='\u{200b}'

Or, if both functions are correct, we should at least document why this odd discrepancy.

[magic-akari]

It looks like is_white_space_single_line should be equivalent to:

fn is_white_space_single_line(c: char) -> bool {
    matches!(c, SP | TAB) || is_irregular_whitespace(c)
}

This is how I initially wrote it. I also noticed the difference with ZWSP.
I attempted to compare it with the ECMAScript specification at https://tc39.es/ecma262/#sec-white-space ,
but neither is_irregular_whitespace nor is_white_space_single_line matched it. For example, NEL (U+0085) isn't listed.
So I simply, inherited TypeScript's logic.

And is_irregular_whitespace was kept because it's used elsewhere.

[overlookmotel]

@Boshen I think we need your expertise on this. Why does is_irregular_whitespace not match ZWSP (\u{200b})?

@magic-akari Boshen is away in Japan, so if you don't want to wait for his reply, could you please:

1. Reimplement is_white_space_single_line as:

// TODO: Unclear why we match `ZWSP` here, and not in `is_irregular_whitespace`.
// https://github.com/oxc-project/oxc/pull/6639
fn is_white_space_single_line(c: char) -> bool {
    matches!(c, SP | TAB | ZWSP) || is_irregular_whitespace(c)
}

2. Move your new constants up with the others, and use them in is_irregular_whitespace i.e. replace '\u{85}' with NEL etc.

3. Open a separate issue about the ZWSP question.

[overlookmotel]

Merge activity

• Oct 17, 10:40 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Oct 17, 10:40 AM EDT: A user added this pull request to the Graphite merge queue.
• Oct 17, 10:45 AM EDT: A user merged this pull request with the Graphite merge queue.

[overlookmotel]

Thanks! I've opened #6646 on the ZWSP question.