Skip to content

Comments

fix(transformer): JSX source calculate correct column when Unicode chars#3615

Merged
graphite-app[bot] merged 1 commit intomainfrom
06-10-fix_transformer_jsx_source_calculate_correct_column_when_unicode_chars
Jun 11, 2024
Merged

fix(transformer): JSX source calculate correct column when Unicode chars#3615
graphite-app[bot] merged 1 commit intomainfrom
06-10-fix_transformer_jsx_source_calculate_correct_column_when_unicode_chars

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Jun 10, 2024

Fix column number in JSX source transform, and add tests.

It was correct in all cases, except for when a Unicode character with code point above 0xFFFF appears earlier on the line.

Such characters are:

  • 4 bytes in UTF-8.
  • 2 characters in UTF-16.
  • 1 char in Rust.

Babel (which we're trying to match) uses count of UTF-16 characters for column number, whereas we were using count of Rust chars.

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 10, 2024

CodSpeed Performance Report

Merging #3615 will not alter performance

Comparing 06-10-fix_transformer_jsx_source_calculate_correct_column_when_unicode_chars (8d237c4) with main (5793ff1)

Summary

✅ 22 untouched benchmarks

@graphite-app
Copy link
Contributor

graphite-app bot commented Jun 11, 2024

Merge activity

…ars (#3615)

Fix column number in JSX source transform, and add tests.

It was correct in all cases, except for when a Unicode character with code point above `0xFFFF` appears earlier on the line.

Such characters are:

* 4 bytes in UTF-8.
* 2 characters in UTF-16.
* 1 `char` in Rust.

Babel (which we're trying to match) uses count of UTF-16 characters for column number, whereas we were using count of Rust `char`s.
@Boshen Boshen force-pushed the 06-10-fix_transformer_do_not_add___source_for_generated_nodes branch from 66ed8ed to 9e8f4d6 Compare June 11, 2024 06:43
@Boshen Boshen force-pushed the 06-10-fix_transformer_jsx_source_calculate_correct_column_when_unicode_chars branch from 326d58e to 8d237c4 Compare June 11, 2024 06:44
@Boshen Boshen changed the base branch from 06-10-fix_transformer_do_not_add___source_for_generated_nodes to main June 11, 2024 06:51
@graphite-app graphite-app bot merged commit 8d237c4 into main Jun 11, 2024
@graphite-app graphite-app bot deleted the 06-10-fix_transformer_jsx_source_calculate_correct_column_when_unicode_chars branch June 11, 2024 06:52
@Dunqing
Copy link
Member

Dunqing commented Jun 11, 2024

We have the same logic here. Seems we need to add a common function to do it.

#[allow(clippy::cast_possible_truncation)]
fn offset_to_position(offset: usize, source_text: &str) -> Option<Position> {
let rope = Rope::from_str(source_text);
let line = rope.try_byte_to_line(offset).ok()?;
let first_char_of_line = rope.try_line_to_char(line).ok()?;
// Original offset is byte, but Rope uses char offset
let offset = rope.try_byte_to_char(offset).ok()?;
let column = offset - first_char_of_line;
Some(Position::new(line as u32, column as u32))
}

This was referenced Jun 12, 2024
Boshen added a commit that referenced this pull request Jun 12, 2024
## [0.14.0] - 2024-06-12

### Breaking

* fix(codegen)!: remove the unecessary 4th argument from `Codegen::new`
(#3640)
* feat(ast)!: make `Trivias` clonable by adding `Arc` (#3638)

### Features

- f6d9ca6 linter: Add `eslint/sort-imports` rule (#3568) (Wang Wenzhe)
- 129f91e span: Port over more methods from TextRange (#3592) (Don
Isaac)

### Bug Fixes

- f8f6d33 ast: Correct `visited_node` attr for strict mode of arrow fns
(#3635) (overlookmotel)
- e6ad3fb diagnostics: Do not print ansi color codes in non-TTYs (#3624)
(Boshen)
- d65202d span: Correct doc comments (#3608) (overlookmotel)
- 35e267b transformer: Arrow function transform use UIDs for `_this`
vars (#3634) (overlookmotel)
- 39bdebc transformer: Arrow func transform maintain scope ID (#3633)
(overlookmotel)
- 5cb7e6a transformer: Arrow func transform use correct spans (#3630)
(overlookmotel)
- 0c4ccb4 transformer: Arrow function transform alter `</this>` (#3627)
(overlookmotel)
- 8d237c4 transformer: JSX source calculate correct column when Unicode
chars (#3615) (overlookmotel)
- 9e8f4d6 transformer: Do not add `__source` for generated nodes (#3614)
(overlookmotel)
- 0fb4c35 transformer: Use UID for JSX source filename var (#3612)
(overlookmotel)

### Performance

- 3a59294 transformer: React display name transform reduce Atom
allocations (#3616) (overlookmotel)
- f4c1389 transformer: Create `Vec` with capacity (#3613)
(overlookmotel)

### Refactor

- 0f92521 ast: Replace recursion with loop (#3626) (overlookmotel)
- 08f1010 ast: Make `AstBuilder` `Copy` (#3602) (overlookmotel)
- 84304b4 linter: Add a `ctx.module_record()` method (#3637) (Boshen)
- f98f777 linter: Add rule fixer (#3589) (Don Isaac)
- e90e6a2 minifier: Make `Prepass` `Copy` (#3603) (overlookmotel)
- 7d61832 semantic: Pass `Rc` by value (#3586) (overlookmotel)
- 89bcbd5 transformer: Move `BoundIdentifier` into helpers (#3610)
(overlookmotel)
- 5793ff1 transformer: Replace `&’a Trivias` with `Rc<Trivias>` (#3580)
(Dunqing)
- 509871f transformer: Comment for unimplemented `spec` option in arrow
fns transform (#3618) (overlookmotel)
- 4b2e3a7 transformer: Fix indentation (#3617) (overlookmotel)
- 3467e3d transformer: Remove outdated comment (#3606) (overlookmotel)
- a799225 transformer: Flatten file structure for React transform
(#3604) (overlookmotel)
- 70f31a8 transformer: Reduce branching in JSX transform (#3596)
(overlookmotel)
- 3ae567d transformer: Remove dead code (#3588) (overlookmotel)
- 60cbdec traverse: `generate_uid_in_root_scope` method (#3611)
(overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants