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

feat(parser): TypeScript 5.2 #811

Merged
merged 25 commits into from
Oct 4, 2023
Merged

feat(parser): TypeScript 5.2 #811

merged 25 commits into from
Oct 4, 2023

Conversation

camc314
Copy link
Collaborator

@camc314 camc314 commented Aug 29, 2023

Closes #786

@CLAassistant
Copy link

CLAassistant commented Aug 29, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added A-parser Area - Parser A-minifier Area - Minifier A-printer Area - Printer A-type-checking Area - Type checking with the Ezno bridge layer labels Aug 29, 2023
@Boshen
Copy link
Member

Boshen commented Aug 29, 2023

Oh wow this is awesome! I'll review this tomorrow since it is getting late here.

One quick question - how do you like the whole experience? Any suggestions for improvements would be greatly appreciated.

@Boshen
Copy link
Member

Boshen commented Aug 29, 2023

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Unfortunately the CLA needs to be signed.

@camc314
Copy link
Collaborator Author

camc314 commented Aug 29, 2023

Oh wow this is awesome! I'll review this tomorrow since it is getting late here.

One quick question - how do you like the whole experience? Any suggestions for improvements would be greatly appreciated.

Thanks!

Not really, it was pretty smooth. It was nice that I didn't have to go diving through the repo and you included all the detail in the issue - link to the TS release notes, instructions for just sync and just coverage.

One of the bits that slowed me down a bit was that:

https://github.com/Boshen/oxc/blob/00ed939cbc302fa288b5051577ebcb79d93e4bed/tasks/coverage/src/suite.rs#L293-L300

so just coverage requires that the ast + hir work was done before parser_typescript.snap is updated/written to. But it makes sense why. I primarily used cargo watch -x "run -p oxc_parser --example parser" for working on the code.

But overall it looks great! You've done a fantastic job with the project so far. And the readme contains everything I could want to know about the project.

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Unfortunately the CLA needs to be signed.

Thanks. Working with me Employer to get it signed 🙂.

@Boshen
Copy link
Member

Boshen commented Aug 30, 2023

so just coverage requires that the ast + hir work was done before parser_typescript.snap is updated/written to. But it makes sense why.

This is a blocker, I should move it to another stage or rethink about HIR.

Thank you for this, things like this will make future contributions easier.

@Boshen
Copy link
Member

Boshen commented Sep 15, 2023

HIR is now removed.

@camc314 Can I continue your work if you are not finding the time?

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 15, 2023

CodSpeed Performance Report

Merging #811 will not alter performance

Comparing camc314:c/ts-5.3 (b9ab6cd) with main (69f2364)

Summary

✅ 18 untouched benchmarks

@camc314
Copy link
Collaborator Author

camc314 commented Sep 19, 2023

HIR is now removed.

@camc314 Can I continue your work if you are not finding the time?

I think this should be ready to go unless I've missed something - but feel free to take over

let me know what you think

@camc314 camc314 marked this pull request as ready for review September 29, 2023 21:32
@Boshen
Copy link
Member

Boshen commented Sep 29, 2023

Looking good, keep up with the good work 👍

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Just some minor nitpicks. This is really good overall.

.gitignore Outdated Show resolved Hide resolved
crates/oxc_ast/src/ast/js.rs Show resolved Hide resolved
crates/oxc_parser/src/js/declaration.rs Outdated Show resolved Hide resolved
crates/oxc_parser/src/js/statement.rs Outdated Show resolved Hide resolved
crates/oxc_parser/src/js/statement.rs Show resolved Hide resolved
@Boshen Boshen merged commit 5b1e1e5 into oxc-project:main Oct 4, 2023
19 checks passed
@Boshen
Copy link
Member

Boshen commented Oct 4, 2023

Thank you so so much for working on this! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-minifier Area - Minifier A-parser Area - Parser A-printer Area - Printer A-type-checking Area - Type checking with the Ezno bridge layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(parser): TypeScript 5.2
3 participants