feat: namespace page-view localStorage key and migrate legacy key - #113
Merged
Conversation
Rename the persisted page-view key from the unprefixed 'mpPageViews' to 'mp-rokt-kit.pageViews', adopting an 'mp-rokt-kit.*' namespace for kit-owned localStorage keys. - Generalize the storage accessors to key-agnostic readJSON/writeJSON/ removeKey so future kit keys can reuse them. All three swallow storage failures (private mode, quota, access denied): persisted page views are a best-effort cache and a failed read/write/remove must never break the caller. - Centralize reads behind loadPageViews(), which runs a one-time, unconditional legacy migration (adopt-if-empty, always-sweep) before reading. - Sweep the legacy key on SessionEnd before clearing the new key. - Confine all migration logic to migrateLegacyPageViewStorage() + LEGACY_PAGE_VIEWS_KEY, marked with a TODO removal date. It works on the opaque stored string (raw localStorage) so malformed legacy data is still swept, not adopted. - Guard the selectPlacements read path: the migration touches localStorage directly and can throw, which must not break placement selection. The stored value stays a bare array; capacity semantics are unchanged.
alexs-mparticle
marked this pull request as ready for review
August 11, 2026 21:30
alexs-mparticle
force-pushed
the
feat/page-view-storage-migration
branch
from
August 11, 2026 21:30
ef4b617 to
726e4cf
Compare
alexs-mparticle
commented
Aug 12, 2026
Route page-view read/write/capture failures through loggingService.log (INFO diagnostic) instead of errorReportingService.report, matching the best-effort-cache posture. writeJSON now returns a success flag so a failed persist is observable without breaking the caller. Add a test pinning the intentional behavior that the targeting-disabled init clear does not sweep the legacy mpPageViews key.
Move the key-agnostic localStorage wrappers (readJSON/writeJSON/removeKey) out of Rokt-Kit.ts into a dedicated src/storage.ts module and import them. Page-view semantics (keys, migration, load/write/clear) stay in Rokt-Kit.ts. - Add test/src/storage.spec.ts with dedicated unit tests for the helpers (happy paths plus getItem/setItem/removeItem throwing). - Trim comments in storage.ts to the one load-bearing "never throw / best-effort" rationale. - Update AGENTS.md: reflect the multi-module source layout, note co-located src/**/*.spec.ts are run by Vitest, and add a no-unnecessary-comments convention.
alexs-mparticle
force-pushed
the
feat/page-view-storage-migration
branch
from
August 12, 2026 15:09
67c15c6 to
7b4c9fb
Compare
crisryantan
reviewed
Aug 12, 2026
crisryantan
reviewed
Aug 12, 2026
crisryantan
reviewed
Aug 12, 2026
crisryantan
approved these changes
Aug 12, 2026
jaissica12
approved these changes
Aug 12, 2026
jaissica12
left a comment
jaissica12
left a comment
Contributor
There was a problem hiding this comment.
lgtm! reminder: can we also make same PR on mP core sdk to align the kit for V3 release
Group kit-owned localStorage state as fields on one JSON object under the 'mp-rokt-kit' key instead of a flat 'mp-rokt-kit.pageViews' key, so future items can share the namespace. Add read/write/removeNamespacedField helpers and migrate the legacy 'mpPageViews' array into the pageViews field.
rmi22186
reviewed
Aug 12, 2026
rmi22186
reviewed
Aug 12, 2026
rmi22186
reviewed
Aug 12, 2026
rmi22186
reviewed
Aug 12, 2026
rmi22186
reviewed
Aug 12, 2026
rmi22186
reviewed
Aug 12, 2026
- migrateLegacyPageViewStorage reads the legacy key once via readJSON instead of a getItem + readJSON pair - tests reuse storage.ts helpers (readJSON/readNamespacedField/ writeNamespacedField) instead of re-implementing localStorage parsing
rmi22186
reviewed
Aug 12, 2026
migrateLegacyPageViewStorage takes the logging service and emits a diagnostic INFO log (retaining the legacy key for retry) instead of throwing on a failed adopt. loadPageViews forwards the logger. With the migration no longer throwing, the purpose-built try/catch blocks in selectPlacements and the SessionEnd handler are removed.
rmi22186
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moves the kit's persisted page-view history out of the unprefixed, top-level
mpPageViewslocalStorage key into a single kit-owned namespace object stored undermp-rokt-kit, with page views held in itspageViewsfield. This establishes onemp-rokt-kitobject where future kit-owned state can live as sibling fields, and adds a one-time migration so existing history survives the move.Changes
mp-rokt-kit→{ "pageViews": [...] }. Future kit-owned state becomes additional fields on the same object rather than new top-level keys.src/storage.ts): key-agnostic JSON read/write/remove helpers, plus namespaced-field helpers that read-modify-write a single field while preserving its siblings. Page-view semantics stay in the callers.mpPageViewshistory is adopted into the new field on first read, and the legacy key is swept. The migration is a self-contained shim marked for removal after 2027-02-11.Migration semantics
The stored field value is a bare array of page views; the only new structure is the enclosing
mp-rokt-kitobject.mpPageViewsmp-rokt-kit.pageViewsNote: with targeting disabled, an orphaned legacy
mpPageViewsmay remain until the next read orSessionEndsweeps it — benign.Tests
Added a legacy-migration suite (adopt, new-wins, no-op,
SessionEndsweep) andsrc/storage.spec.tscovering the storage helpers (round-trip, malformed JSON, access/quota errors, sibling preservation, namespace cleanup). Existing page-view tests repointed to the new key/field shape.Verify: lint · build · tests all pass.