refactor(client): drop Accounts._unstoreLoginToken monkey-patch#40483
refactor(client): drop Accounts._unstoreLoginToken monkey-patch#40483ggazzo wants to merge 1 commit into
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ 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). (3)
WalkthroughThe PR moves logout and token cleanup from a Meteor Accounts monkey-patch to DDP SDK callbacks, adds a ChangesLogout Flow Migration
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs:
Suggested reviewers:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor/sdk-tracker-easy-autoruns #40483 +/- ##
======================================================================
+ Coverage 69.62% 69.64% +0.01%
======================================================================
Files 3319 3318 -1
Lines 122006 122002 -4
Branches 21823 21783 -40
======================================================================
+ Hits 84952 84973 +21
+ Misses 33726 33701 -25
Partials 3328 3328
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/jira ARCH-2116 |
The remaining client-side Accounts._unstoreLoginToken calls fell into two roles: a monkey-patch that piggybacked on Meteor's token-clear hook to run side effects on logout, and direct calls used to manually wipe the stored credentials. Both are replaceable now that #40442 landed sdk.account.onLogout and #40479 landed the SDK storage helper. - Delete apps/meteor/client/meteor/overrides/unstoreLoginToken.ts (and its barrel entry). CachedStoresManager.ts registers clearAllCachesOnLogout via getDdpSdk().account.onLogout at module init, which fires on the same uid-undefined transition Meteor's monkey-patch was capturing. - AuthenticationProvider's unstoreLoginToken context method collapses from a monkey-patch + restore pair into `(callback) => getDdpSdk().account.onLogout(callback)`. Same return shape (callback -> unsubscribe), so useUnstoreLoginToken consumers stay intact. - AuthenticationProvider.wipeLocalAuth and the auth-error branch in ddpSdk.ts call removeStoredItem for USER_ID / LOGIN_TOKEN / LOGIN_TOKEN_EXPIRES — the three keys Meteor's _unstoreLoginToken clears. Adds LOGIN_TOKEN_EXPIRES to STORAGE_KEYS so storage.ts owns the full set. No behaviour change: same localStorage keys cleared, same logout cleanup ordering. The Accounts import stays in AuthenticationProvider.tsx and ddpSdk.ts because callLoginMethod / loggingIn / getLoginToken / the onEmailVerificationLink + onPageLoadLogin bridges still use it.
ggazzo
force-pushed
the
refactor/drop-unstore-login-token-monkey-patch
branch
from
1512465 to
47df1e1
Compare
ggazzo
changed the base branch from
develop
to
refactor/sdk-tracker-easy-autoruns
1 participant
Summary
The remaining client-side
Accounts._unstoreLoginTokencalls fell into two roles: a monkey-patch that piggybacked on Meteor's token-clear hook to run side effects on logout, and direct calls used to manually wipe the stored credentials. Both are replaceable now that #40442 landedsdk.account.onLogoutand #40479 landed the SDK storage helper.apps/meteor/client/meteor/overrides/unstoreLoginToken.ts(and its barrel entry).CachedStoresManager.tsnow registersclearAllCachesOnLogoutviagetDdpSdk().account.onLogoutat module init, which fires on the sameuid → undefinedtransition Meteor's monkey-patch was capturing.AuthenticationProvider'sunstoreLoginTokencontext method collapses from a monkey-patch + restore pair into(callback) => getDdpSdk().account.onLogout(callback). Same return shape (callback → unsubscribe), souseUnstoreLoginTokenconsumers stay intact.AuthenticationProvider.wipeLocalAuthand the auth-error branch inddpSdk.tsnow callremoveStoredItemforUSER_ID/LOGIN_TOKEN/LOGIN_TOKEN_EXPIRES— the three keys Meteor's_unstoreLoginTokenclears. AddsLOGIN_TOKEN_EXPIREStoSTORAGE_KEYSsostorage.tsowns the full set.No behaviour change: same localStorage keys cleared, same logout cleanup ordering. The
Accountsimport stays inAuthenticationProvider.tsxandddpSdk.tsbecausecallLoginMethod/loggingIn/getLoginToken/ theonEmailVerificationLink+onPageLoadLoginbridges still use it (out of scope here).Test plan
yarn eslinton the 5 changed files + the deleted file: 0 errors.tsc --noEmitonapps/meteoris clean (no new errors in changed files).Meteor.userId/Meteor.loginToken/Meteor.loginTokenExpiresare removed.clearAllCachesOnLogoutfired.ddpSdk.tsclears the credentials and the router falls back to/login.useIframeLoginListenerre-trigger after logout still works (it depends onuseUnstoreLoginToken, which now bridges throughsdk.account.onLogout).Summary by CodeRabbit
Refactor
Bug Fixes
Task: ARCH-2136