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(jest-leak-detector): use FinalizationRegistry when it exists to get rid of external dependency #12973

merged 1 commit into from Jun 30, 2022


Copy link

@enrilzhou enrilzhou commented Jun 28, 2022


In the past, the leak detector required external dependency 'weak-napi', a C module, to be installed and work. Now since node has supported the FinalizationRegistry from ECMA, we can use this native object when available to reduce external dependency.

Test plan

Could keep using the existing unit test.

SimenB approved these changes Jun 28, 2022
Copy link

@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.

Woah, didn't know about this api. Awesome, thanks! 👍

@enrilzhou enrilzhou force-pushed the enril/leak-detector-use-native branch 3 times, most recently from a12280c to d6853ec Compare June 30, 2022 14:58
@enrilzhou enrilzhou force-pushed the enril/leak-detector-use-native branch from d6853ec to 61f3366 Compare June 30, 2022 16:06
@SimenB SimenB merged commit 51fa619 into facebook:main Jun 30, 2022
89 of 90 checks passed
Copy link

SimenB commented Jul 13, 2022

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 Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants