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

Update NodeEnvironment to provide dispose global symbols #14888

Merged

Conversation

notaphplover
Copy link
Contributor

@notaphplover notaphplover commented Feb 8, 2024

Summary

This PR aims to solve #14874.

As said in the issue, since typescript 5.2, Explicit Resource Management is being supported in Typescript. Latest stable NodeJS versions include Symbol.dispose and Symbol.asyncDispose, essential to pollyfill this feature. Exposing them in the jest node env would allow to properly test modules using this feature.

The vm node API generates by default a global object with standard properties. I think it makes sense the node env provides dispose Symbols if the host runtime is providing them in order to use NodeJS supported features relying in those symbols.

Test plan

  • Added a unit test to test the host global Symbol is set as the vm global Symbol.

Copy link

netlify bot commented Feb 8, 2024

Deploy Preview for jestjs ready!

Name Link
🔨 Latest commit d95c2a1
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/65c613116981d60008898982
😎 Deploy Preview https://deploy-preview-14888--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

linux-foundation-easycla bot commented Feb 8, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@notaphplover notaphplover changed the title feat: update NodeEnvironment to create a context with host Symbol global Update NodeEnvironment to provide dispose global symbols Feb 8, 2024
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

this is great, thanks!

@SimenB SimenB linked an issue Feb 9, 2024 that may be closed by this pull request
@SimenB SimenB merged commit 8bbe2a3 into jestjs:main Feb 9, 2024
16 of 17 checks passed
@notaphplover notaphplover deleted the feat/update-node-js-env-with-node-js-symbol branch February 9, 2024 16:28
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Symbol.dispose and Symbol.asyncDispose are undefined
2 participants