fix(devtools): handle PiP open failures by resetting persisted state #9983
Conversation
🦋 Changeset detectedLatest commit: 300488d The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
WalkthroughThis patch release introduces error handling for Picture-in-Picture (PiP) popup failures in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 300488d
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-devtools/src/contexts/PiPContext.tsx (1)
29-29: LGTM! Clean custom error class.The
PipOpenErrorclass correctly extendsErrorand enables type-safe error handling viainstanceofchecks.Optional: Add name property for better error identification in logs
-class PipOpenError extends Error {} +class PipOpenError extends Error { + constructor(message: string) { + super(message) + this.name = 'PipOpenError' + } +}This would make the error more identifiable in stack traces, but it's purely cosmetic.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/warm-drinks-itch.md(1 hunks)packages/query-devtools/src/contexts/PiPContext.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-devtools/src/contexts/PiPContext.tsx (1)
packages/query-devtools/src/constants.ts (1)
PIP_DEFAULT_HEIGHT(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (3)
.changeset/warm-drinks-itch.md (1)
1-5: LGTM! Standard changeset format.The changeset correctly documents this as a patch release with a clear description of the fix.
packages/query-devtools/src/contexts/PiPContext.tsx (2)
61-65: Excellent improvement to error handling.Throwing
PipOpenErrorinstead of a genericErrorenables precise downstream error handling. The error message is clear and actionable, guiding users to allow popups.
139-152: Perfect fix for the PiP state inconsistency issue.The error handling correctly addresses the problem described in the PR:
- When popup blocking prevents PiP from opening, the persisted
pip_openandopenflags are reset tofalse- This restores the devtools button visibility, allowing users to retry without the awkward workaround
- The
instanceofcheck ensures only popup failures are handled gracefully while other errors propagate- The early
returnprevents cascading failures and infinite effect loopsThe implementation is clean and the state reset ensures consistency between the persisted state and actual window state.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9983 +/- ##
===========================================
- Coverage 45.89% 21.24% -24.65%
===========================================
Files 200 42 -158
Lines 8437 2419 -6018
Branches 1943 607 -1336
===========================================
- Hits 3872 514 -3358
+ Misses 4116 1667 -2449
+ Partials 449 238 -211
🚀 New features to boost your workflow:
|
1 participant
🎯 Changes
The existing implementation of handling persistent PiP state leaves room for the state to be in an awkward state.
Closing the app while PiP mode is active will cause devtools to try to reopen in PiP mode. The issue with this is that when the browser permissions for pop-ups are disactive, it will fail. The devtools state will reflect that devtools is open and no longer display the devtools button while devtools itself is not open for obvious reasons.
Although the user can workaround this by enabling pop-ups, switch off the PiP state, and disable pop-ups, I thought this was cumbersome. My proposal is to reset the state so that devtools thinks it's closed. So the user can re-open it with the default method.
Screencast.from.2025-12-19.22-42-57.webm
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.