Skip to content

refactor(parser): remove lookahead usage in parsing arrow function expressions#11220

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions
May 22, 2025
Merged

refactor(parser): remove lookahead usage in parsing arrow function expressions#11220
graphite-app[bot] merged 1 commit intomainfrom
05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented May 21, 2025

Replaces all usage of the lexer lookahead functions like nth_kind, so that we only use checkpoints and rewinding, which is aligned with the TypeScript compiler implementation. This also will allow us to remove the lookahead field from the lexer in the future.

@github-actions github-actions bot added A-parser Area - Parser C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels May 21, 2025
Copy link
Member Author

camchenry commented May 21, 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.

@codspeed-hq
Copy link

codspeed-hq bot commented May 21, 2025

CodSpeed Instrumentation Performance Report

Merging #11220 will degrade performances by 13.46%

Comparing 05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions (def05bc) with 05-20-refactor_parser_use_checkpoints_instead_of_peek_at_in_is_un_parenthesized_async_arrow_function_worker_ (a9dbf0a)

Summary

❌ 1 regressions
✅ 37 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
mangler[cal.com.tsx] 3 ms 3.5 ms -13.46%

@camchenry
Copy link
Member Author

@Boshen is this sort of snapshot update expected? It looks like the parser error message has changed here, where before we just had "unexpected token" and now it says we were looking for a "=>", which feels better? But I'm not sure if it's correct or not, since I'm less familiar with the parser changes.

image

@camchenry camchenry force-pushed the 05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions branch from 269e829 to c1f6380 Compare May 21, 2025 15:41
@camchenry
Copy link
Member Author

Actually, it looks like this is correct I think. This is the same error that TypeScript will emit:

image

https://www.typescriptlang.org/play/?#code/BQOjEMEoAIGpoIwCgg

@camchenry camchenry force-pushed the 05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions branch from c1f6380 to 597969f Compare May 21, 2025 15:46
@camchenry camchenry marked this pull request as ready for review May 21, 2025 15:46
@Boshen
Copy link
Member

Boshen commented May 22, 2025

@Boshen is this sort of snapshot update expected? It looks like the parser error message has changed here, where before we just had "unexpected token" and now it says we were looking for a "=>", which feels better? But I'm not sure if it's correct or not, since I'm less familiar with the parser changes.

image

Yes! We expect more alignment to tsc from making these changes.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 22, 2025
Copy link
Member

Boshen commented May 22, 2025

Merge activity

…pressions (#11220)

- towards #11194

Replaces all usage of the lexer lookahead functions like `nth_kind`, so that we only use checkpoints and rewinding, which is aligned with the TypeScript compiler implementation. This also will allow us to remove the lookahead field from the lexer in the future.
@graphite-app graphite-app bot force-pushed the 05-20-refactor_parser_use_checkpoints_instead_of_peek_at_in_is_un_parenthesized_async_arrow_function_worker_ branch from c1b83c8 to a9dbf0a Compare May 22, 2025 02:59
@graphite-app graphite-app bot force-pushed the 05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions branch from 597969f to def05bc Compare May 22, 2025 02:59
Base automatically changed from 05-20-refactor_parser_use_checkpoints_instead_of_peek_at_in_is_un_parenthesized_async_arrow_function_worker_ to main May 22, 2025 03:05
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 22, 2025
@graphite-app graphite-app bot merged commit def05bc into main May 22, 2025
26 checks passed
@graphite-app graphite-app bot deleted the 05-21-refactor_parser_remove_lookahead_usage_in_parsing_arrow_function_expressions branch May 22, 2025 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants