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(react-spring-raf): update react-spring deps to resolve raf overload #2282
Conversation
🦋 Changeset detectedLatest commit: f2c2f6d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
I guess we should just double check any animations we have in components to make sure they don't break, but yeah, easy easy!
✔️ Deploy Preview for paste-theme-designer ready! 🔨 Explore the source changes: 7b54348 🔍 Inspect the deploy log: https://app.netlify.com/sites/paste-theme-designer/deploys/6228dc820317dd000a3b2a2c 😎 Browse the preview: https://deploy-preview-2282--paste-theme-designer.netlify.app |
✔️ Deploy Preview for paste-docs ready! 🔨 Explore the source changes: 7b54348 🔍 Inspect the deploy log: https://app.netlify.com/sites/paste-docs/deploys/6228dc82346b9b0008116130 😎 Browse the preview: https://deploy-preview-2282--paste-docs.netlify.app |
Size Change: +1.59 kB (0%) Total Size: 518 kB
ℹ️ View Unchanged
|
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. Latest deployment of this branch, based on commit f2c2f6d:
|
I checked out Alert Dialog, Modal, Disclosure, Toast, and the website landing page (which uses our animation library) - all appears to be 👍🏼 |
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Is there any blocker to merge this PR? |
Hi @askel4dd, yes, none of our tests will pass with this update. It has highlighted a significant memory leak in the test suite. Currently working through those right now before getting back to this. |
bae25a2
to
4c12a4c
Compare
✅ Deploy Preview for paste-theme-designer ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for paste-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
2958822
to
1ebd735
Compare
732b934
to
352e7b7
Compare
packages/paste-core/components/skeleton-loader/__tests__/index.spec.tsx
Outdated
Show resolved
Hide resolved
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.
I'm not sure if we (can) support keyframe animations in Box currently, so this looks perfectly fine.
When tested before/after on any component using @twilio-paste/animation-library (such as Alert Dialog, Badge, Toast, etc) I see the unnecessary calls to RAF no longer happening.