Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Mar 17, 2025

Re-order the fields of TemplateElement, moving tail to last. This means fields are in same order as ESTree, withou having to manually re-order them.

The field order makes no real difference here, since tail field is not visited, so I figure might as well align with ESTree.

Copy link
Member Author

overlookmotel commented Mar 17, 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-parser Area - Parser A-minifier Area - Minifier A-ast Area - AST C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Mar 17, 2025
@overlookmotel overlookmotel marked this pull request as ready for review March 17, 2025 07:36
@overlookmotel overlookmotel force-pushed the 03-17-refactor_ast_reorder_fields_of_templateelement_ branch from f935178 to 118a88a Compare March 17, 2025 07:37
@overlookmotel overlookmotel force-pushed the 03-17-feat_ast_estree_order_ts_fields_last_by_default branch from 3b6899b to 182da49 Compare March 17, 2025 07:37
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 17, 2025

CodSpeed Performance Report

Merging #9821 will not alter performance

Comparing 03-17-refactor_ast_reorder_fields_of_templateelement_ (e99a27a) with main (db946e6)

Summary

✅ 33 untouched benchmarks

@overlookmotel overlookmotel requested a review from Boshen March 17, 2025 08:20
@overlookmotel
Copy link
Member Author

overlookmotel commented Mar 17, 2025

@Boshen Just checking it's not a big problem to make small changes to AST like this? It is a breaking change as it alters the order of params to AstBuilder::template_element.

We don't have to make this change - ESTree serialization was working fine already. It's just a bit of "tidy up".

@graphite-app graphite-app bot changed the base branch from 03-17-feat_ast_estree_order_ts_fields_last_by_default to graphite-base/9821 March 17, 2025 08:27
@graphite-app graphite-app bot force-pushed the 03-17-refactor_ast_reorder_fields_of_templateelement_ branch from 118a88a to 7838c35 Compare March 17, 2025 08:33
@graphite-app graphite-app bot force-pushed the graphite-base/9821 branch from 182da49 to db946e6 Compare March 17, 2025 08:33
@graphite-app graphite-app bot changed the base branch from graphite-base/9821 to main March 17, 2025 08:34
@graphite-app graphite-app bot force-pushed the 03-17-refactor_ast_reorder_fields_of_templateelement_ branch from 7838c35 to e99a27a Compare March 17, 2025 08:34
@Boshen Boshen merged commit 3d17860 into main Mar 18, 2025
27 checks passed
@Boshen Boshen deleted the 03-17-refactor_ast_reorder_fields_of_templateelement_ branch March 18, 2025 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-minifier Area - Minifier A-parser Area - Parser 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.

3 participants