Skip to content

refactor(ecma/lexer): split lexer of parser #10377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 16, 2025

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented Apr 15, 2025

We are not ensure each token is correct during parse js/ts code, so let's split parse/lexer into single crates and this crate can ensure lexer is correctly

@bvanjoi bvanjoi requested review from a team as code owners April 15, 2025 12:38
Copy link

changeset-bot bot commented Apr 15, 2025

🦋 Changeset detected

Latest commit: 6b0e4fc

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codspeed-hq bot commented Apr 15, 2025

CodSpeed Performance Report

Merging #10377 will improve performances by 3.05%

Comparing bvanjoi:rm-token-contexts (6b0e4fc) with main (773d19c)

Summary

⚡ 1 improvements
✅ 140 untouched benchmarks
🆕 11 new benchmarks
⁉️ 11 dropped benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 es/lexer/angular N/A 12.2 ms N/A
🆕 es/lexer/backbone N/A 1.7 ms N/A
🆕 es/lexer/cal-com N/A 33.8 ms N/A
🆕 es/lexer/colors N/A 44.2 µs N/A
🆕 es/lexer/jquery N/A 8.8 ms N/A
🆕 es/lexer/jquery mobile N/A 13.6 ms N/A
🆕 es/lexer/mootools N/A 7.1 ms N/A
🆕 es/lexer/three N/A 43.2 ms N/A
🆕 es/lexer/typescript N/A 236.2 ms N/A
🆕 es/lexer/underscore N/A 1.4 ms N/A
🆕 es/lexer/yui N/A 7.2 ms N/A
es/lints/libs/d3 28.6 ms 27.8 ms +3.05%
⁉️ es/lexer/angular 12.2 ms N/A N/A
⁉️ es/lexer/backbone 1.7 ms N/A N/A
⁉️ es/lexer/cal-com 33.8 ms N/A N/A
⁉️ es/lexer/colors 44.2 µs N/A N/A
⁉️ es/lexer/jquery 8.8 ms N/A N/A
⁉️ es/lexer/jquery mobile 13.6 ms N/A N/A
⁉️ es/lexer/mootools 7.1 ms N/A N/A
⁉️ es/lexer/three 43.2 ms N/A N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@bvanjoi
Copy link
Contributor Author

bvanjoi commented Apr 16, 2025

It's surprising that these failing test cases parse correctly in the playground environment, yet were passing in our earlier test suite.

For instance, the failing test case in test262::error_reporting::fail::0ff3826356c94f67.js containing ({function} = 0) was parsed successfully but generated no error reports before.

@kdy1 kdy1 self-assigned this Apr 16, 2025
@kdy1 kdy1 added this to the Planned milestone Apr 16, 2025
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

CI failed, but I guess it's #10378, right? Please feel free to fix CI in any direction - except removing the whole tests.

I agree that it's a problem, but I just didn't bother to think about the solution deeply.

@bvanjoi bvanjoi force-pushed the rm-token-contexts branch from 4f625f4 to e246e9a Compare April 16, 2025 07:07
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Apr 16, 2025

Please feel free to fix CI in any direction

The issue occurs because swc_ecma_lexer doesn't receive the feature="verify" flag. Moving is_reserved_word to swc_ecma_parser resolves this.

@kdy1
Copy link
Member

kdy1 commented Apr 16, 2025 via email

@bvanjoi bvanjoi force-pushed the rm-token-contexts branch from e246e9a to 4690101 Compare April 16, 2025 09:53
@bvanjoi
Copy link
Contributor Author

bvanjoi commented Apr 16, 2025

Appreciate the clarification! The design pattern makes sense now.

Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Thanks!

@kdy1 kdy1 merged commit 3ef2bd1 into swc-project:main Apr 16, 2025
34 checks passed
@kdy1 kdy1 modified the milestones: Planned, v1.11.22 Apr 23, 2025
@swc-project swc-project locked as resolved and limited conversation to collaborators May 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants