Skip to content

refactor(estree): remove regular expression types from ESTree AST#10855

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-05-refactor_estree_remove_regular_expression_types_from_estree_ast
May 7, 2025
Merged

refactor(estree): remove regular expression types from ESTree AST#10855
graphite-app[bot] merged 1 commit intomainfrom
05-05-refactor_estree_remove_regular_expression_types_from_estree_ast

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented May 7, 2025

Pattern and other types in oxc_regular_expression are not part of ESTree AST. Don't implement ESTree, and don't generate raw transfer deserializers for these types. None of that code is actually used.

#10834 moved Pattern to an optional field of RegExpPattern, which is skipped in ESTree serialization, which enables now removing it from ESTree AST entirely.

This reduces the scope of ESTree serialization.

Copy link
Member Author

overlookmotel commented May 7, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@github-actions github-actions bot added A-ast Area - AST C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels May 7, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented May 7, 2025

CodSpeed Instrumentation Performance Report

Merging #10855 will not alter performance

Comparing 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast (daba0a7) with main (05bf1be)

Summary

✅ 36 untouched benchmarks

@overlookmotel overlookmotel marked this pull request as ready for review May 7, 2025 09:55
@overlookmotel overlookmotel requested a review from leaysgur as a code owner May 7, 2025 09:55
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

I'll let @leaysgur take a final look.

@graphite-app graphite-app bot changed the base branch from 05-05-refactor_ast_simplify_regexppattern_ to graphite-base/10855 May 7, 2025 10:05
@graphite-app graphite-app bot force-pushed the graphite-base/10855 branch from 11f56a6 to ad4fbf4 Compare May 7, 2025 10:12
@graphite-app graphite-app bot force-pushed the 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast branch from 97be93d to 983a06c Compare May 7, 2025 10:12
@graphite-app graphite-app bot changed the base branch from graphite-base/10855 to main May 7, 2025 10:13
@graphite-app graphite-app bot force-pushed the 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast branch from 983a06c to 5465876 Compare May 7, 2025 10:13
Copy link
Member

@leaysgur leaysgur left a comment

Choose a reason for hiding this comment

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

I don't think there's any particular issue as a decision.
(BTW, I'm probably the only person in the world who uses this, so I might need to think about how to handle. 😅)

I'll also update #10846 accordingly.

Copy link
Member

Boshen commented May 7, 2025

Merge activity

  • May 7, 7:50 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 7:50 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • May 7, 11:57 AM UTC: The Graphite merge queue couldn't merge this PR because it had conflicts with the trunk branch.
  • May 7, 8:49 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 7, 8:55 AM EDT: overlookmotel added this pull request to the Graphite merge queue.
  • May 7, 8:55 AM EDT: Merged by the Graphite merge queue.

graphite-app bot pushed a commit that referenced this pull request May 7, 2025
…0855)

`Pattern` and other types in `oxc_regular_expression` are not part of ESTree AST. Don't implement `ESTree`, and don't generate raw transfer deserializers for these types. None of that code is actually used.

#10834 moved `Pattern` to an optional field of `RegExpPattern`, which is skipped in ESTree serialization, which enables now removing it from ESTree AST entirely.

This reduces the scope of ESTree serialization.
@graphite-app graphite-app bot force-pushed the 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast branch from 5465876 to 776d7e1 Compare May 7, 2025 11:51
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 7, 2025
@Boshen
Copy link
Member

Boshen commented May 7, 2025

oops the other branch got merged first.

…0855)

`Pattern` and other types in `oxc_regular_expression` are not part of ESTree AST. Don't implement `ESTree`, and don't generate raw transfer deserializers for these types. None of that code is actually used.

This reduces the scope of ESTree serialization.
@overlookmotel overlookmotel force-pushed the 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast branch from 776d7e1 to daba0a7 Compare May 7, 2025 12:47
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label May 7, 2025
@graphite-app graphite-app bot merged commit daba0a7 into main May 7, 2025
28 checks passed
@graphite-app graphite-app bot deleted the 05-05-refactor_estree_remove_regular_expression_types_from_estree_ast branch May 7, 2025 12:55
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 7, 2025
@ArnaudBarre
Copy link
Contributor

Thanks for cleaning generated types 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST 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.

regular_expression AST: Modifier type is included in Node union but doesn't extend Span

4 participants