Conversation
WalkthroughThe PR upgrades Jest dependencies (29.x to 30.x), refactors Jest configuration typing, and introduces window.location and window.history as injectable services. OnboardingContainer is updated to consume these services instead of directly accessing window objects, eliminating the need for global state cleanup in tests. Changes
Sequence DiagramsequenceDiagram
participant Test
participant DI as DI Container
participant Component
participant MockServices as Mocked Services
Note over Test,MockServices: Before: Direct window mutation + cleanup
Test->>Component: Pass setup dependencies
Component->>window: Access window.location/history directly
Test->>Test: Cleanup (restore window state)
Note over Test,MockServices: After: Injected services, no cleanup needed
Test->>MockServices: Create windowLocation & windowHistory mocks
Test->>DI: Provide mocked services
DI->>Component: Inject windowLocation & windowHistory
Component->>MockServices: Access location/history via services
Test->>Test: No cleanup required
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🧰 Additional context used📓 Path-based instructions (4)**/*.spec.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Files:
apps/{deploy-web,provider-console}/**/*.spec.tsx📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-07-21T08:25:07.474ZApplied to files:
📚 Learning: 2025-07-21T08:25:07.474ZApplied to files:
📚 Learning: 2025-07-21T08:24:27.953ZApplied to files:
📚 Learning: 2025-07-21T08:25:07.474ZApplied to files:
📚 Learning: 2025-07-21T08:25:07.474ZApplied to files:
📚 Learning: 2025-07-21T08:25:07.474ZApplied to files:
📚 Learning: 2025-07-21T08:24:24.269ZApplied to files:
📚 Learning: 2025-07-11T10:46:43.711ZApplied to files:
📚 Learning: 2025-07-27T12:16:08.566ZApplied to files:
🧬 Code graph analysis (1)apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
⏰ 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). (15)
🔇 Additional comments (5)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| const { user } = d.useUser(); | ||
| const { data: paymentMethods = [] } = d.usePaymentMethodsQuery({ enabled: !!user?.stripeCustomerId }); | ||
| const { analyticsService, urlService, authService, chainApiHttpClient, deploymentLocalStorage, appConfig, errorHandler } = d.useServices(); | ||
| const { analyticsService, urlService, authService, chainApiHttpClient, deploymentLocalStorage, appConfig, errorHandler, windowLocation, windowHistory } = |
There was a problem hiding this comment.
non-blocking: perhaps it makes more sense to just use window so that we wouldn't need to add every window prop in the future?
| const { analyticsService, urlService, authService, chainApiHttpClient, deploymentLocalStorage, appConfig, errorHandler, windowLocation, windowHistory } = | |
| const { analyticsService, urlService, authService, chainApiHttpClient, deploymentLocalStorage, appConfig, errorHandler, windowLocation, window } = |
There was a problem hiding this comment.
from another hand, it will be harder to mock the whole window in tests. Though possible with mockDeep<Window>(). Honestly, I think this is a rare case, for example localStorage we need to encapsulate separately.
but the main reason behind this change is that I just needed to fix it for new jsdom version which doesn't allow to redefine location and history on window. Ideally we should use router for both cases, not sure whether react router exposes this information
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2179 +/- ##
=======================================
Coverage 46.70% 46.71%
=======================================
Files 1019 1019
Lines 28929 28931 +2
Branches 7515 7516 +1
=======================================
+ Hits 13512 13515 +3
+ Misses 15103 15026 -77
- Partials 314 390 +76
🚀 New features to boost your workflow:
|
2 participants
Why
To use newer version of jsdom which contains newer browser API (e.g.,
AbortSignal.any)Summary by CodeRabbit
Chores
Refactor