Skip to content

Comments

Add support for using and await using#23

Merged
adams85 merged 29 commits intomasterfrom
explicit-resource-management
Jul 12, 2025
Merged

Add support for using and await using#23
adams85 merged 29 commits intomasterfrom
explicit-resource-management

Conversation

@lahma
Copy link
Collaborator

@lahma lahma commented Jul 3, 2025

Here to show some progress, might need some help to finalize but hopefully this is more than nothing. Branch pushed directly to this repo for easier checkout if you want to.

Copy link
Owner

@adams85 adams85 left a comment

Choose a reason for hiding this comment

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

Here to show some progress, might need some help to finalize but hopefully this is more than nothing.

I could just take a quick look now but I can say it's promising! Please allow me some more time to dig into it properly.

Branch pushed directly to this repo for easier checkout if you want to.

👍

lahma and others added 9 commits July 5, 2025 09:04
Co-authored-by: adams85 <31276480+adams85@users.noreply.github.com>
Co-authored-by: adams85 <31276480+adams85@users.noreply.github.com>
Co-authored-by: adams85 <31276480+adams85@users.noreply.github.com>
@lahma
Copy link
Collaborator Author

lahma commented Jul 6, 2025

I've completed the work on Jint side and all tests are passing there too now.

@lahma lahma marked this pull request as ready for review July 6, 2025 14:40
@adams85
Copy link
Owner

adams85 commented Jul 6, 2025

Thank you for taking your time! I revised your work and made some corrections/improvements.

Unfortunately, there are still some rough edges to polish off:

  • The double lookahead in isUsingKeyword is buggy in the original implementation. Acorn incorrectly accepts nonsense like async () => { await using\u0065 a = b } at the moment. I'll need to look into this, maybe discuss it with the acorn guys.
  • Error reporting doesn't follow V8 in all cases (e.g. in bare switch cases). I'll set this straight too.
  • Would be nice to port tests from acorn. Working on it. Plus, we should add some tests for the new AllowTopLevelUsing option.

To be continued.

@lahma
Copy link
Collaborator Author

lahma commented Jul 6, 2025

Thanks, much appreciated. I know I create extra burden but hopefully at least something to build upon 😅

@adams85 adams85 force-pushed the explicit-resource-management branch 2 times, most recently from 37424ac to 86b0bd1 Compare July 6, 2025 22:15
@adams85 adams85 force-pushed the explicit-resource-management branch from 86b0bd1 to 2596869 Compare July 6, 2025 22:17
@adams85
Copy link
Owner

adams85 commented Jul 6, 2025

Come on, this has to be done anyway - the sooner, the better! :)

@adams85 adams85 force-pushed the explicit-resource-management branch from 4ec3ad8 to 5f71778 Compare July 9, 2025 09:50
@adams85
Copy link
Owner

adams85 commented Jul 12, 2025

Okay, I think this is complete now. @lahma Could you take a final look?

@lahma
Copy link
Collaborator Author

lahma commented Jul 12, 2025

Quite a lot of changed files, but checking the commits seems really good, thanks for pushing the final 90% 😄 All tests passing on Jint side so I think it's good to go 🚀

@lahma
Copy link
Collaborator Author

lahma commented Jul 12, 2025

I'm also totally fine if you want to credit this to yourself in commit log instead of me, you did the actual work 💪🏻

@adams85
Copy link
Owner

adams85 commented Jul 12, 2025

Nope, this was a full-blown co-production. :D I wouldn't even have started implementing this if you hadn't taken the first steps...

I'm going to release this soon, along with some additional upstream improvements plus some minor performance-related tweaks.

@adams85 adams85 merged commit 25966a1 into master Jul 12, 2025
3 checks passed
@adams85 adams85 deleted the explicit-resource-management branch July 12, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants