Skip to content

feat(transformer/class-properties): transform ChainExpression#7575

Merged
graphite-app[bot] merged 1 commit intomainfrom
11-30-feat_transformer_class-properties_transform_chainexpressionb
Dec 4, 2024
Merged

feat(transformer/class-properties): transform ChainExpression#7575
graphite-app[bot] merged 1 commit intomainfrom
11-30-feat_transformer_class-properties_transform_chainexpressionb

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Dec 2, 2024

Fully support transforming ChainExpression in class properties. Our implementation has some differences in execution order compared to Babel, but we have passed all execution-related tests.

@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 2, 2024

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.

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request labels Dec 2, 2024
Copy link
Member Author

Dunqing commented Dec 2, 2024

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

@Dunqing Dunqing changed the title feat(transformer/class-properties): transform ChainExpressionb feat(transformer/class-properties): transform ChainExpression Dec 2, 2024
@Dunqing Dunqing marked this pull request as ready for review December 2, 2024 02:30
@Dunqing Dunqing marked this pull request as draft December 2, 2024 02:30
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 3, 2024

CodSpeed Performance Report

Merging #7575 will not alter performance

Comparing 11-30-feat_transformer_class-properties_transform_chainexpressionb (c793d71) with main (bd9d38a)

Summary

✅ 11 untouched benchmarks
🆕 18 new benchmarks

Benchmarks breakdown

Benchmark main 11-30-feat_transformer_class-properties_transform_chainexpressionb Change
🆕 codegen[checker.ts] N/A 25.4 ms N/A
🆕 codegen_sourcemap[checker.ts] N/A 76.9 ms N/A
🆕 isolated-declarations[vue-id.ts] N/A 729.8 ms N/A
🆕 lexer[RadixUIAdoptionSection.jsx] N/A 24.1 µs N/A
🆕 lexer[antd.js] N/A 22.3 ms N/A
🆕 lexer[cal.com.tsx] N/A 5.5 ms N/A
🆕 lexer[checker.ts] N/A 13.2 ms N/A
🆕 lexer[pdf.mjs] N/A 3.6 ms N/A
🆕 parser[RadixUIAdoptionSection.jsx] N/A 87.8 µs N/A
🆕 parser[antd.js] N/A 107.4 ms N/A
🆕 parser[cal.com.tsx] N/A 29.6 ms N/A
🆕 parser[checker.ts] N/A 54.1 ms N/A
🆕 parser[pdf.mjs] N/A 17.5 ms N/A
🆕 semantic[RadixUIAdoptionSection.jsx] N/A 74.4 µs N/A
🆕 semantic[antd.js] N/A 117.2 ms N/A
🆕 semantic[cal.com.tsx] N/A 28.1 ms N/A
🆕 semantic[checker.ts] N/A 71.6 ms N/A
🆕 semantic[pdf.mjs] N/A 19.2 ms N/A

@Dunqing Dunqing marked this pull request as ready for review December 4, 2024 09:18
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is a monster! I've not been through it in detail. Have just added a couple of commits for nits, and merging. Will look at it a bit more properly later.

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Dec 4, 2024
Copy link
Member

overlookmotel commented Dec 4, 2024

Merge activity

  • Dec 4, 11:50 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 4, 11:54 AM EST: A user added this pull request to the Graphite merge queue.
  • Dec 4, 11:57 AM EST: A user merged this pull request with the Graphite merge queue.

Fully support transforming ChainExpression in class properties. Our implementation has some differences in execution order compared to Babel, but we have passed all execution-related tests.
@overlookmotel overlookmotel force-pushed the 11-30-feat_transformer_class-properties_transform_chainexpressionb branch from 59e95ad to c793d71 Compare December 4, 2024 16:53
@graphite-app graphite-app bot merged commit c793d71 into main Dec 4, 2024
@graphite-app graphite-app bot deleted the 11-30-feat_transformer_class-properties_transform_chainexpressionb branch December 4, 2024 16:57
overlookmotel added a commit that referenced this pull request Dec 4, 2024
Follow-on after #7575. By convention we use backticks for code snippets in doc comments.
Dunqing pushed a commit that referenced this pull request Dec 5, 2024
Follow-on after #7575. Small optimization. Replace recursive function calls with a loop.
Dunqing pushed a commit that referenced this pull request Dec 5, 2024
…te` of chain expression into `transform_unary_expression` (#7655)

Follow-on after #7575. Pure refactor. Move all logic for transforming `delete <chain expression>` into the handler `transform_unary_expression`. Aim is to keep logic in one place and keep the main visitor as simple as possible.
Dunqing pushed a commit that referenced this pull request Dec 5, 2024
…n in static prop initializers (#7656)

Follow-on after #7575. Annoyingly, we have to transform private fields in static prop initializers separately, to make sure the stack of private properties is in sync while visiting them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants