Skip to content

refactor(transform): transformer use Traverse#3182

Merged
Dunqing merged 1 commit intomainfrom
05-06-refactor_transform_transformer_use_Traverse_
May 8, 2024
Merged

refactor(transform): transformer use Traverse#3182
Dunqing merged 1 commit intomainfrom
05-06-refactor_transform_transformer_use_Traverse_

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented May 6, 2024

Sliced off from #3152.

This switches the transformer over to use Traverse instead of VisitMut.

This is incomplete - scopes are not implemented yet. At present, no transforms use scopes anyway, so all tests pass, but regardless I don't think should be merged until the implementation is complete.

@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label May 6, 2024
Copy link
Member Author

overlookmotel commented May 6, 2024

@overlookmotel overlookmotel marked this pull request as ready for review May 6, 2024 18:34
@overlookmotel overlookmotel marked this pull request as draft May 6, 2024 18:35
@codspeed-hq
Copy link

codspeed-hq bot commented May 6, 2024

CodSpeed Performance Report

Merging #3182 will degrade performances by 3.69%

Comparing 05-06-refactor_transform_transformer_use_Traverse_ (4222e67) with main (762677e)

Summary

❌ 2 regressions
✅ 25 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main 05-06-refactor_transform_transformer_use_Traverse_ Change
transformer[checker.ts] 345.6 ms 356.4 ms -3.04%
transformer[pdf.mjs] 105 ms 109 ms -3.69%

@Dunqing Dunqing merged commit be958ce into main May 8, 2024
@Dunqing Dunqing deleted the 05-06-refactor_transform_transformer_use_Traverse_ branch May 8, 2024 09:18
@overlookmotel
Copy link
Member Author

We discussed and agreed to merge as is without scopes support, and to add that later on in a separate PR.

Dunqing pushed a commit that referenced this pull request May 8, 2024
…up lookup (#3183)

Sliced off from #3152.

Re-implement `transform-react-display-name` using bottom-up lookup
provided by `Traverse` trait.

This fixes the 1 remaining failing test case for this plugin (see
#2937).

`Traverse` is not complete yet (see #3182), so this is also not ready to
merge yet.
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