Skip to content
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

perf(es/parser): Rewrite parser #9313

Draft
wants to merge 424 commits into
base: dev
Choose a base branch
from
Draft

perf(es/parser): Rewrite parser #9313

wants to merge 424 commits into from

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Jul 22, 2024

Description:

I'm going to rewrite the ES parser of SWC because

  • I didn't know anything about compiler or parser theories at the time of writing the parser (ecmascript parser #1).
  • @Boshen and oxc team investigated a lot and proved that the parser can be much faster than now. (Wonderful job, it's really nice)

Related issue (if exists):

@kdy1 kdy1 added this to the Planned milestone Jul 22, 2024
@kdy1 kdy1 self-assigned this Jul 22, 2024
Copy link

changeset-bot bot commented Jul 22, 2024

⚠️ No Changeset found

Latest commit: bcd03cb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@Boshen
Copy link
Contributor

Boshen commented Jul 22, 2024

Please be aware in oxc, there is a second ast pass for ast path, symbols and scopes to complete all syntax errors.

We also use the latest conformance suite from https://github.com/tc39/test262, you may need to consider replacing the outdated one you are using.

Copy link

codspeed-hq bot commented Jul 22, 2024

CodSpeed Performance Report

Merging #9313 will not alter performance

Comparing kdy1:new-parser (0fda55e) with main (a6d1db8)

Summary

✅ 178 untouched benchmarks

@d3lm
Copy link

d3lm commented Jul 22, 2024

Would be cool if there was a way for it to also produce a red green syntax tree to allow modifications to the code that preserves formatting.

@Boshen Any thoughts on this?

I personally have a lot of use cases where I need to parse some module and then transform the code. The problem is that with an AST, since it's lossy in terms of formatting, any transformations to the tree means that if you re-print it in the end you'd loose the formatting of the module.

It'd be cool if it used a lossless syntax tree instead of an AST.

@Boshen
Copy link
Contributor

Boshen commented Jul 22, 2024

Would be cool if there was a way for it to also produce a red green syntax tree to allow modifications to the code that preserves formatting.

@Boshen Any thoughts on this?

I personally have a lot of use cases where I need to parse some module and then transform the code. The problem is that with an AST, since it's lossy in terms of formatting, any transformations to the tree means that if you re-print it in the end you'd loose the formatting of the module.

It'd be cool if it used a lossless syntax tree instead of an AST.

CST is out of scope for swc / oxc. Take a look at https://crates.io/crates/biome_parser for CST.

@d3lm
Copy link

d3lm commented Jul 22, 2024

Yea I was aware of Biome's parser that's inspired by the Rust Analyzer's parser I guess. Thanks!

Copy link

Report too large to display inline

View full report↗︎

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSourceCI
Install scripts npm/core-js@2.6.12
  • Install script: postinstall
  • Source: node -e "try{require('./postinstall')}catch(e){}"
🚫

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore npm/core-js@2.6.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants