Skip to content

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Mar 3, 2025

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Further clean up the left-value parsing. This PR can be reviewed by commits.

@JLHwung JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: parser labels Mar 3, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Mar 3, 2025

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58802

@JLHwung JLHwung force-pushed the lval-parsing-cleanup branch from cb806d3 to 1996132 Compare March 3, 2025 01:43
"throws": "Unexpected token (1:7)"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The input is async(x?), previously we threw on ) now we throw on ?.

// These tokens cannot start an expression, so if one of them follows
// ? then we are probably in an arrow function parameters list and we
// don't parse the conditional expression.
if (
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo Mar 3, 2025

Choose a reason for hiding this comment

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

Nice, did you check how this improves perf?

Copy link
Contributor Author

@JLHwung JLHwung Mar 3, 2025

Choose a reason for hiding this comment

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

Just added a benchmark case, here is the result on my machine:

baseline 256 nested conditional expressions with typescript: 7_828 ops/sec ±0.4% (0.128ms)
baseline 512 nested conditional expressions with typescript: 3_740 ops/sec ±0.2% (0.267ms)
baseline 1024 nested conditional expressions with typescript: 1_875 ops/sec ±0.16% (0.533ms)
baseline 2048 nested conditional expressions with typescript: 0 ops/sec ±0% (0ms)
current 256 nested conditional expressions with typescript: 8_108 ops/sec ±0.42% (0.123ms)
current 512 nested conditional expressions with typescript: 4_036 ops/sec ±0.29% (0.248ms)
current 1024 nested conditional expressions with typescript: 1_977 ops/sec ±0.23% (0.506ms)
current 2048 nested conditional expressions with typescript: 947 ops/sec ±0.45% (1.056ms)

It is 8% faster than the baseline. It turns out the baseline can't handle 2048 nested conditional expressions within parentheses, probably it hits certain memory limit, while the new approach works well.

@JLHwung JLHwung force-pushed the lval-parsing-cleanup branch from c732e54 to e51529e Compare March 3, 2025 20:31
@JLHwung JLHwung force-pushed the lval-parsing-cleanup branch from e51529e to 565b6d5 Compare March 3, 2025 20:32
@nicolo-ribaudo nicolo-ribaudo merged commit fb1e134 into babel:main Mar 3, 2025
55 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the lval-parsing-cleanup branch March 3, 2025 21:23
laine-hallot pushed a commit to laine-hallot/uwu-parser that referenced this pull request Mar 31, 2025
* cleanup

* refactor: simplify checkProto implementation

* rename test

* refactor: avoid second scan in type cast transform

* refactor: simplify ts parseConditional

The current logic is taken from the flow plugin

* add benchmark cases
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 3, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants