Skip to content

Comments

fix(ast/estree): fix raw deser for FormalParameter#10548

Merged
leaysgur merged 1 commit intomainfrom
04-22-fix_ast_estree_fix_raw_deser_for_formalparameter_
Apr 23, 2025
Merged

fix(ast/estree): fix raw deser for FormalParameter#10548
leaysgur merged 1 commit intomainfrom
04-22-fix_ast_estree_fix_raw_deser_for_formalparameter_

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Apr 22, 2025

Follow-on after #10534. Fix the raw deser implementation for FormalParameter to handle TSParameterProperty.

This involves moving some of the logic into a converter for FormalParameter, instead of doing it all in FormalParameters converter.

@github-actions github-actions bot added A-ast Area - AST C-bug Category - Bug labels Apr 22, 2025
Copy link
Member Author


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.

@overlookmotel overlookmotel marked this pull request as ready for review April 22, 2025 16:36
@overlookmotel
Copy link
Member Author

overlookmotel commented Apr 22, 2025

@leaysgur Can you please check I have this correct? Or does this foul things up for other types which include FormalParameters e.g. TSCallSignatureDeclaration, TSMethodSignature, and TSFunctionType?

(oh man I really hate the lengths we have having to go to here!)

@overlookmotel overlookmotel requested a review from leaysgur April 22, 2025 16:39
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 22, 2025

CodSpeed Instrumentation Performance Report

Merging #10548 will not alter performance

Comparing 04-22-fix_ast_estree_fix_raw_deser_for_formalparameter_ (9b7c937) with main (7e71282)

Summary

✅ 36 untouched benchmarks

@leaysgur
Copy link
Member

leaysgur commented Apr 23, 2025

Thanks for always cleaning up after me!
(EXCUSE: I also tried to implement it directly for FormalParameter at first, but for some reason I couldn't... 🙉)

From my quick check, there seemed to be no problems.
I did find differences due to other areas than this one, but I will address them later.

@leaysgur leaysgur merged commit 8eb3c0a into main Apr 23, 2025
30 checks passed
@leaysgur leaysgur deleted the 04-22-fix_ast_estree_fix_raw_deser_for_formalparameter_ branch April 23, 2025 03:12
@overlookmotel
Copy link
Member Author

No excuses required! The syntax is so horrible it makes it very difficult. Anyway, we agreed not to worry about raw_deser until later anyway. It's nice if we can keep it in sync as we go, but we don't have to.

overlookmotel pushed a commit that referenced this pull request Apr 23, 2025
I happened to find this when checking #10548 👀 

(It might not have been necessary to fix it right away, but since I
found it 😅)
Boshen added a commit that referenced this pull request Apr 23, 2025
## [0.66.0] - 2025-04-23

### Features

- 7d5ad7d parser: Report error when `import type { type }` is used
(#10528) (camc314)
- 6e40fac parser: Report error when `export type { type }` is used
(#10524) (camc314)

### Bug Fixes

- 43ad4e9 ast: Box `this_param` in `TSCallSignatureDeclaration` (#10558)
(Yuji Sugiura)
- 8eb3c0a ast/estree: Fix raw deser for `FormalParameter` (#10548)
(overlookmotel)
- 3ebf220 codegen: Generate missing `type` in `export type {} from
'mod'` (#10539) (Boshen)
- f19b287 estree: Add `TSParameterProperty` (#10534) (Yuji Sugiura)
- 7284f16 isolated-declarations: Leading comments of
`ExportDefaultDeclaration` and `TSExportAssignment` appear in incorrect
places (#10559) (Dunqing)
- 4c316a5 isolated_declarations: Fix broken snapshot files (#10561)
(Boshen)
- 84742d8 transformer/module_runner: Fix export default live binding
(#10560) (hi-ogawa)
- 6c115c7 transformer/module_runner: Fix execution order of re-export
(#10362) (hi-ogawa)
- b9d84b0 transformer/typescript: Remove `StringLiteral::raw` when
rewriting extensions (#10554) (overlookmotel)

### Performance

- 6a045c8 codegen: Speed up printing `Directive`s (#10551)
(overlookmotel)
- 3418ae5 transformer/class-properties: Re-use
`InstanceInitializerVisitor` (#10543) (overlookmotel)
- 7fcf0ac transformer/typescript: Reduce allocations renaming extensions
(#10555) (overlookmotel)

### Documentation

- 1a9530c transformer/class-properties: Fix typo (#10542)
(overlookmotel)

### Refactor

- 7e71282 transformer/class-properties: Correct outdated comment
(#10546) (overlookmotel)
- 596978b transformer/class-properties: Simplify to get `constructor`
method only once (#10493) (Dunqing)
- f13fe0a transformer/class-properties: Defer re-parenting initializers
scopes until all instance properties are transformed (#10495) (Dunqing)
- c83dad6 transformer/class-properties: Streamline handling scope of
instance property initializer (#10492) (Dunqing)

### Styling

- e10dfc8 traverse: Remove excess whitespace (#10544) (overlookmotel)

### Testing

- 227febf allocator: Ignore a slow doc test (#10521) (Dunqing)
- 14bb2be codegen: Add more tests for strings (#10552) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants