Skip to content

refactor(transformer/class-properties): use duplicate_object in transform_expression_to_wrap_nullish_check#7664

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check
Dec 5, 2024
Merged

refactor(transformer/class-properties): use duplicate_object in transform_expression_to_wrap_nullish_check#7664
graphite-app[bot] merged 1 commit intomainfrom
12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Dec 5, 2024

The duplicate_object is great, it has handled unbound identifiers and this expressions, so we can remove a lot of code. But it is still a little bit annoying because we need two object here, although we can use clone_in it needs a special logic for Expression::Identifier.

@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 5, 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.

@Dunqing Dunqing changed the title refactor(transformer/class-properties) use duplicate_object in transform_expression_to_wrap_nullish_check refactor(transformer/class-properties) use duplicate_object in transform_expression_to_wrap_nullish_check Dec 5, 2024
Copy link
Member Author

Dunqing commented Dec 5, 2024

@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label Dec 5, 2024
@Dunqing Dunqing changed the title refactor(transformer/class-properties) use duplicate_object in transform_expression_to_wrap_nullish_check refactor(transformer/class-properties): use duplicate_object in transform_expression_to_wrap_nullish_check Dec 5, 2024
@github-actions github-actions bot added the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Dec 5, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 5, 2024

CodSpeed Performance Report

Merging #7664 will not alter performance

Comparing 12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check (8883968) with main (f029090)

Summary

✅ 29 untouched benchmarks

@overlookmotel overlookmotel force-pushed the 12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check branch from cd04ec0 to f787b71 Compare December 5, 2024 17:40
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.

Rebased on main and made a couple of small tweaks.

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

overlookmotel commented Dec 5, 2024

Merge activity

  • Dec 5, 12:41 PM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 5, 12:45 PM EST: A user added this pull request to the Graphite merge queue.
  • Dec 5, 12:51 PM EST: A user merged this pull request with the Graphite merge queue.

…ansform_expression_to_wrap_nullish_check` (#7664)

The `duplicate_object` is great, it has handled unbound identifiers and `this` expressions, so we can remove a lot of code. But it is still a little bit annoying because we need two `object` here,  although we can use `clone_in` it needs a special logic for `Expression::Identifier`.
@overlookmotel overlookmotel force-pushed the 12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check branch from f787b71 to 8883968 Compare December 5, 2024 17:46
@graphite-app graphite-app bot merged commit 8883968 into main Dec 5, 2024
@graphite-app graphite-app bot deleted the 12-05-refactor_transformer_class-properties_use_duplicate_object_in_transform_expression_to_wrap_nullish_check branch December 5, 2024 17:51
Dunqing pushed a commit that referenced this pull request Dec 5, 2024
…od (#7685)

Follow-up after #7664.

Generalize `duplicate_object` to produce however many duplicates are required. Use it in `transform_expression_to_wrap_nullish_check`.

This should be slightly more efficient, and will make it fool-proof if we expand `duplicate_object` later to handle other `Expression` types.
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-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants