Skip to content

docs(settings): Correct JwtTokenCache storage comment - #20962

Open
nshirley wants to merge 1 commit into
mainfrom
fix-jwt-token-cache-comment
Open

docs(settings): Correct JwtTokenCache storage comment#20962
nshirley wants to merge 1 commit into
mainfrom
fix-jwt-token-cache-comment

Conversation

@nshirley

@nshirley nshirley commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Because

  • The doc comment on JwtTokenCache stated that MFA JWTs "will be held in page memory" and would not survive hard navigates. That is not how the class behaves: the tokens are backed by local storage and intentionally persist across page refreshes.
  • The comment also advised swapping static state for session storage "if edge cases arise," which would undo deliberate behavior that test infrastructure already relies on.

This pull request

  • Rewrites the JwtTokenCache doc comment in packages/fxa-settings/src/lib/cache.ts to state that tokens are backed by local storage and survive hard navigates and refreshes.
  • Describes static state as what it actually is: an in-memory cache of the stored value, lazily hydrated on first read and written through on every mutation.
  • Drops the stale suggestion to move to session storage.

No behavior change — comment only.

Issue that this pull request solves

Closes: N/A

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Other information (Optional)

No Jira ticket — comment-only correction with no behavior change.

Supporting evidence for the corrected wording:

  • cache.ts:15 builds the store via Storage.factory('localStorage'), which resolves to new Storage(win.localStorage) (storage.ts:160-161).
  • The state setter (cache.ts:287-290) calls storage.set(...), and setToken/removeToken/clearTokens all reassign state, so every mutation is written through.
  • packages/functional-tests/lib/testAccountTracker.ts:666-690 reads and writes localStorage['__fxa_storage.mfa_token_cache'] directly, so cross-navigation persistence is already depended upon.

Because:
* The comment claimed JWTs are held in page memory, but they are backed
  by local storage and intentionally persist across refreshes.

This commit:
* Describes `static state` as a write-through cache of local storage.
@nshirley
nshirley marked this pull request as ready for review August 3, 2026 20:15
Copilot AI review requested due to automatic review settings August 3, 2026 20:15
@nshirley
nshirley requested a review from a team as a code owner August 3, 2026 20:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the JwtTokenCache doc comment in fxa-settings to better describe how MFA JWTs are stored and cached, aligning documentation with the cache’s actual persistence model.

Changes:

  • Rewrites the JwtTokenCache comment to describe local-storage-backed persistence across refreshes/navigations.
  • Clarifies that static state acts as an in-memory cache hydrated from persistent storage and written-through on mutations.

Comment on lines +257 to +259
* Tokens are backed by local storage so they survive hard navigates and page
* refreshes. `static state` is an in-memory cache of that stored value; it is
* lazily hydrated on first read and written through on every mutation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants