-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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 jest-leak-detector
package to be compatible with the new Node version
#8686
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove the conditional require
now, which'll necessitate typings.
Since we're changing the signature, that also means this is a breaking change... I see CI fails for node 6, but since it's breaking we'll just drop node 6 anyways
be7a7c0
to
05cb207
Compare
@SimenB @jeysal DT was released type for |
Hmm, something seems to be off with CI - Circle runs on your account instead of Jest's. Not sure how that happened |
how I fix it? :< |
Noticed Circle doing the same for #8689, perhaps it only worked there because I have the necessary permissions. Maybe it works if you remove your fork from the list of projects that Circle builds. |
@jeysal I have removed it from list in my CircleCI. Please trigger run build agian! |
I don't know if we can rerun if there's no build for this PR in the history. Can you try to git commit --amend --no-edit && git push -f |
05cb207
to
d282052
Compare
I did it and waiting to CircleCI re-run |
That worked 👌 |
Either leak detection has improved or it's now wrong... See the node 10 failure (where we run |
|
|
this PR will be waiting to test on |
Yeah, we'll merge this when we start landing breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
many thanks for reviewed! ^^ |
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. |
At #8397, we know
weak
package doesn't work in Node version 12 and we useweak-napi
package instead ofweak
in new version.weak-napi
have interface likeweak
package but it still have some differences suck as:The consequence of this is that the
isLeaking
function injest-leak-detector
package return value must be converted into a promise to wait a bit time.Fixes #8397