-
-
Notifications
You must be signed in to change notification settings - Fork 567
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
fix(tests): add memory leak tests #2508
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Preview in LiveCodesLatest commit: 3e3bffb
See documentations for usage instructions. |
Thanks, I will have a closer look later, but:
So, it means this doesn't reveal the regression with |
yeah, it seems to be already a problem in main |
tests/vanilla/memoryleaks.test.ts
Outdated
;(() => { | ||
const objAtom = atom({}) | ||
detector = new LeakDetector(store.get(objAtom)) | ||
})() |
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.
Hm, I somehow thought function closure would be necessary, but I think I got something wrong. Let me update my PR.
Hi, I updated the main branch. Would you please pull it and adjust this PR so that the diff becomes minimal? |
Fixed by #2533 |
Related Issues or Discussions
#2477
Summary
Fails with store and store2