feat(stores): client-side persistence for page stores (persist "local"|"session")#358
Conversation
Add a `persist "local"|"session"` modifier to page store declarations. The store runtime hydrates from localStorage/sessionStorage on load and on SPA navigation, writes the store's declared fields on change, syncs across tabs via the storage event, exposes window.__gowdkStores.clear(name), and discards persisted state when the store's embedded schema hash no longer matches the store's shape (so a struct change never restores stale data). Only the store's declared fields persist — never component state, props, or computed values. New diagnostics: page_store_persist_scope_invalid (error), page_store_persist_secret_field (warning), and page_store_persist_key_conflict (warning, cross-page same-name divergent-shape). Refs #352. Follow-ups: #354 (WASM islands), #355 (component store fields), #356 (declarative clear), #357 (LSP).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d22a75e9c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if prior.signature == signature { | ||
| continue // same shape sharing one key across routes is intended |
There was a problem hiding this comment.
When two pages declare the same persisted store name with the same struct shape but different scopes, e.g. persist "local" on one route and persist "session" on another, this shape-only check treats them as intentional sharing and emits no diagnostic. The browser registry is keyed only by store name (registry.persist[name]), so SPA navigation keeps whichever scope initialized first while a full reload uses the current page's scope, making persistence behavior depend on navigation history. Include the persist scope in this conflict check or diagnose mismatched scopes.
Useful? React with 👍 / 👎.
| registry.init = (name, state, persist) => { | ||
| if (!name || registry.stores[name]) return; |
There was a problem hiding this comment.
If a user SPA-navigates from a page with an unpersisted store cart to another page that declares the same store as persist "local" or persist "session", hydrate() calls init() but this early return prevents recording the new persist config and seed. Subsequent island updates only change memory and never write browser storage, while directly loading the second route does persist, so persistence depends on navigation history. Update the persist metadata for existing stores or reject mixed persisted/unpersisted declarations for the same name.
Useful? React with 👍 / 👎.
| for _, field := range resolved.Fields { | ||
| if !looksLikeSecretFieldName(field.Name) { | ||
| continue |
There was a problem hiding this comment.
This warning only scans top-level fields, but persistence writes the entire value of each top-level field, so a persisted store like Profile ui.Profile where Profile contains an exported Token or Password field is still written to localStorage/sessionStorage without any page_store_persist_secret_field diagnostic. Since ResolveStruct already records nested field paths in FieldTypes, include those nested names in the secret-field check to avoid silently persisting credential-shaped data.
Useful? React with 👍 / 👎.
Three same-class silent-fallback issues where persisted page-store behavior depended on navigation history or skipped a check: 1. Mismatched persist scopes (validate_stores.go): two pages persisting the same store name with the same shape but different scopes (local vs session) share one storage key. The runtime keeps whichever scope initialized first, so the effective backend depended on nav order. Now warns with page_store_persist_scope_conflict (registered + explain entry). 2. Dropped persist config on re-hydration (runtime_assets.go): when a store was first declared without persistence and a later route declared it persisted, registry.init early-returned and silently discarded the persist config, so writes never reached storage. init now adopts the persist config for an existing-but-unpersisted store and restores the saved value, independent of navigation order. 3. Nested secret fields (validate_page.go): the secret-field check only scanned top-level fields, so a persisted store nesting e.g. Profile.Token wrote a credential-shaped value to browser storage with no warning. It now scans nested field paths from FieldTypes. Tests: scope-conflict + nested-secret compiler tests, a node store-runtime step asserting persistence adoption across SPA navigation, and a nested-secret fixture type. CHANGELOG, docs (syntax/components), and explain updated. Full non-example suite green.
fix(stores): close persistence fallbacks flagged in PR #358 review
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44826e52f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
| const seed = Object.assign(Object.create(null), state || {}); | ||
| registry.fields[name] = Object.keys(seed); |
There was a problem hiding this comment.
When a persisted store's Go type has a declared field that is omitted from the init JSON, such as a zero-valued json:",omitempty" field, deriving registry.fields from Object.keys(seed) means that field is never included in projectFields or decodePersisted; after an island sets the field, reloads and cross-tab sync drop it even though the feature documents that the store's declared fields persist. Generate the persisted field list from the resolved store contract (including JSON names) rather than from the initial seed payload.
Useful? React with 👍 / 👎.
| parts := make([]string, 0, len(resolved.Fields)) | ||
| for _, field := range resolved.Fields { | ||
| parts = append(parts, field.Name+":"+field.Type) | ||
| } |
There was a problem hiding this comment.
For same-name persisted stores, this signature only compares top-level Go field names/types, but the runtime version hash also changes for nested FieldTypes and seed JSON keys. If two pages use the same store name with identical top-level fields but different nested structs or JSON tags, navigation still discards each other's saved value while page_store_persist_key_conflict is not emitted; build the conflict signature from the same shape inputs as storeSchemaHash.
Useful? React with 👍 / 👎.
| {Code: "page_store_persist_key_conflict", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted stores share a storage key but have different shapes"}, | ||
| {Code: "page_store_persist_scope_conflict", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted stores share a storage key but use different scopes"}, | ||
| {Code: "page_store_persist_scope_invalid", Area: "stores", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page store persist scope is not \"local\" or \"session\""}, | ||
| {Code: "page_store_persist_secret_field", Area: "stores", Stability: StabilityExperimental, Severity: SeverityWarning, Summary: "persisted page store field resembles a secret"}, |
There was a problem hiding this comment.
These new diagnostics are emitted by validation, but docs/reference/diagnostic-codes.md has no page_store_persist_* entries (checked with rg page_store_persist docs/reference/diagnostic-codes.md). The repo diagnostic contract requires every emitted code to be listed in both the registry and the reference docs, so users and editor/CI integrations relying on the diagnostic reference cannot discover these warnings/errors. Add the four new codes to the reference table.
Useful? React with 👍 / 👎.
| // saved value so persistence does not depend on which route loaded first. | ||
| // A conflicting scope is kept first-wins and reported at build time by | ||
| // page_store_persist_scope_conflict, so navigation cannot thrash storage. | ||
| if (persist && persist.scope && persist.key && persist.version && !registry.persist[name]) { |
There was a problem hiding this comment.
When SPA navigation reaches another page that declares the same persisted store name with a different shape/version (the warning-only page_store_persist_key_conflict case), this existing-store branch returns as soon as registry.persist[name] is already set, so the new seed/version is ignored and the new page's islands keep using the previous page's store fields until a full reload. Compare the incoming persist.version and reset/discard on mismatch instead of bailing out.
Useful? React with 👍 / 👎.
| if (!event || !event.key) return; | ||
| Object.keys(registry.persist).forEach((name) => { | ||
| const config = registry.persist[name]; | ||
| if (!config || config.key !== event.key) return; |
There was a problem hiding this comment.
When a persist "session" store is active and another tab or script changes localStorage under the same gowdk:store:<name> key (for example stale data from a previous local-scoped version), this handler only compares the key and applies the localStorage payload to the session-backed store. Check event.storageArea against storageFor(config.scope) before decoding/removing so local and session stores cannot cross-contaminate.
Useful? React with 👍 / 👎.
| if store.Name == "" || (store.Persist != "local" && store.Persist != "session") { | ||
| continue |
There was a problem hiding this comment.
This cross-page check skips any store that is not already persisted, but the runtime explicitly adopts persistence when a later route declares the same store name. If a user SPA-navigates from an unpersisted cart store with one shape to a persisted cart store with another shape, the adoption path reuses the first route's registry.fields, so the persisted route reads/writes the wrong field set and may never persist its declared fields; warn on same-name shape conflicts even when only one declaration is persisted, or reset the registry fields/seed during adoption.
Useful? React with 👍 / 👎.
| <script type="application/json" data-gowdk-store="cart" | ||
| data-gowdk-persist="local" | ||
| data-gowdk-persist-key="gowdk:store:cart" | ||
| data-gowdk-persist-version="…">{"count":0}</script> |
There was a problem hiding this comment.
The example shows the persisted seed as {"count":0}, but building this exact example emits {"Count":0} because the state-init runner reflects Go field names instead of honoring json:"count". That makes the new example misleading for users trying to verify the generated output and reinforces a JSON-tag behavior the runtime does not currently provide; either update the example output or make store seed generation honor the struct tags.
Useful? React with 👍 / 👎.
- runtime: re-seed a store when a later route changes its field set or
persisted version (covers cross-route version conflicts and
unpersisted->persisted shape adoption) instead of bailing on the
existing-store branch
- runtime: ignore storage events from the other storage area so local and
session stores sharing a key cannot cross-contaminate
- compiler: fold the full nested FieldTypes into the persist conflict
signature so it tracks the same Go-shape inputs as storeSchemaHash
- docs: list the page_store_persist_* codes in the diagnostic reference
- example: seed serializes by Go field name ({"Count":0}); drop the
misleading json tag
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 225e8d3d1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof window !== 'undefined' && window.__gowdkStores && window.__gowdkStores.hydrate) { | ||
| window.__gowdkStores.hydrate(); |
There was a problem hiding this comment.
When SPA navigation first reaches a page that introduces stores.js, activateNewScripts can execute the new store runtime and island bundle out of order, while island bundles auto-mount as soon as they execute and read window.__gowdkStores during mount. If the island executes first, it marks the root mounted without a store subscription or persisted hydrate, and the later hydrate/mount call here cannot repair it because mounted roots are skipped; force the store script to run/hydrate before island scripts can auto-mount.
Useful? React with 👍 / 👎.
| if store.Persist == "" { | ||
| return nil |
There was a problem hiding this comment.
For store cart ... persist "", the parser stores the empty scope in store.Persist, which is the same sentinel used for no modifier, so this early return silently treats the declaration as unpersisted instead of emitting page_store_persist_scope_invalid. An explicit empty scope should be diagnosed or the AST/IR should track that the persist clause was present.
Useful? React with 👍 / 👎.
| const shapeChanged = | ||
| !sameFieldSet(registry.fields[name] || [], fields) || | ||
| (hasPersist && prior && prior.version !== persist.version); |
There was a problem hiding this comment.
When SPA navigation reaches an unpersisted store first and later reaches a persisted declaration with the same top-level field names but a different nested shape, prior is nil so this condition treats the incoming version as unchanged. The adoption path below keeps the old seed/field metadata, so on a fresh browser storage slot the later route's islands can mount without fields from their declared persisted seed; compare or record the incoming version when persistence is first adopted, or re-seed from the current declaration before restoring storage.
Useful? React with 👍 / 👎.
| // Cross-tab sync: when another tab writes a persisted store, mirror the value | ||
| // here and notify islands. We never write back, so tabs cannot loop. | ||
| if (typeof window.addEventListener === "function") { | ||
| window.addEventListener("storage", (event) => { |
There was a problem hiding this comment.
For persist "session", this cross-tab path cannot see another tab's writes because sessionStorage is partitioned by top-level browser tab rather than origin-only like localStorage (MDN). The docs now promise that persisted stores sync across tabs, but session-backed stores will only sync within the same page session/iframes; either scope the promise to local stores or use a shared broadcast channel for session stores.
Useful? React with 👍 / 👎.
| if (typeof window.addEventListener === "function") { | ||
| window.addEventListener("storage", (event) => { |
There was a problem hiding this comment.
When SPA navigation leaves all store pages, the head swap removes the stores.js script tag while window.__gowdkStores remains alive; navigating later to another store page makes activateNewScripts treat stores.js as new and execute this block again. Each visit registers another storage listener, so every cross-tab write can notify subscribed islands multiple times and rerun render/effect work; keep a listener-attached flag on the registry before adding this handler.
Useful? React with 👍 / 👎.
- runtime: re-seed when a later route first adopts persistence (not only on field-set/version change) so a fresh storage slot uses the persisted route's declared seed instead of an earlier unpersisted route's - runtime: only mirror cross-tab storage events for persist "local" stores; sessionStorage is partitioned per tab, so session stores are tab-local - runtime: attach the storage listener once per registry so SPA re-activation of stores.js cannot register duplicate listeners - clientrt: run + hydrate the store runtime before island bundles during SPA navigation (ordered, non-async) so islands never mount before the store - parser/ast/ir: track PersistSet so an explicit empty scope (persist "") is diagnosed as page_store_persist_scope_invalid instead of treated as unpersisted - docs/changelog: scope the cross-tab sync promise to local stores Covered by parser, validator, clientrt-source, and node store-runtime tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7381f696a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parts := make([]string, 0, len(resolved.FieldTypes)) | ||
| for name, fieldType := range resolved.FieldTypes { | ||
| parts = append(parts, name+":"+fieldType) |
There was a problem hiding this comment.
When two pages persist the same store name with structs that have the same Go field names/types but different JSON tags, this signature remains identical because it only hashes resolved.FieldTypes. buildgen.storeSchemaHash also folds in the seed's wire JSON keys, so those two pages will generate different persist versions for the same gowdk:store:<name> key and discard each other's saved data on navigation, but page_store_persist_key_conflict will not warn. Include the seed wire keys (or otherwise mirror storeSchemaHash) in this conflict signature so json-tag-only shape changes are diagnosed.
Useful? React with 👍 / 👎.
| const shapeChanged = | ||
| !sameFieldSet(registry.fields[name] || [], fields) || | ||
| (hasPersist && (!prior || prior.version !== persist.version)); |
There was a problem hiding this comment.
If a persisted route is visited before a later SPA route that declares the same store name and field set without persist, hasPersist is false so this condition does not reinitialize the registry and leaves registry.persist[name] from the earlier route. Subsequent registry.set calls on the non-persisted route still write to local/session storage, so omitting persist only works when that route is the first one loaded. Treat persistence presence as part of the reinit decision or clear the prior config when the current seed has no persist metadata.
Useful? React with 👍 / 👎.
| if (!registry.storageListenerAttached && typeof window.addEventListener === "function") { | ||
| registry.storageListenerAttached = true; | ||
| window.addEventListener("storage", (event) => { | ||
| if (!event || !event.key) return; |
There was a problem hiding this comment.
When another same-origin tab calls localStorage.clear(), the browser sends a storage event with a null key, but this guard returns before resetting any persisted local stores. That leaves this tab showing stale in-memory store values even though the persisted backing data has been removed, so cross-tab clearing only works for per-key removeItem calls such as __gowdkStores.clear(name). Handle the null-key clear event by resetting all local-scoped persisted stores for the matching storage area.
Useful? React with 👍 / 👎.
- runtime: when a later SPA route declares the same store WITHOUT persist, drop the prior route's persist config so set() stops writing to storage the current route never opted into (in-memory value stays shared) - runtime: handle the null-key "storage" event from another tab's bulk Storage.clear() by resetting local-scoped stores to their seed, not only per-key removeItem; ignore clears of the other storage area Codex flagged a third item (fold seed wire keys into the persist conflict signature) that does not apply: the seed encoder serializes by Go field name and ignores json tags, so a json-tag-only change yields an identical seed and an identical storeSchemaHash -- no divergent version, nothing to warn about. Covered by node store-runtime tests #15 (omit-persist) and #16 (bulk clear).
1 participant
Summary
persist "local"|"session"modifier to pagestoredeclarations — the store-based approach evaluated in [Proposal] Client-side persistence for page stores (persist "local"|"session") — worth it? #352 (Option C).store cart ui.CartState = ui.NewCartState() persist "local"— the generated store runtime:localStorage/sessionStorageon load and on SPA navigation (re-scans seeds on content swap),storageevent,window.__gowdkStores.clear(name)(drop persisted copy + reset to seed),page_store_persist_scope_invalid(error),page_store_persist_secret_field(warning),page_store_persist_key_conflict(warning — same persisted name, divergent shape across pages). All havegowdk explainentries.Persist→ validation/diagnostics → store-seed persist config + schema hash → store runtime. Formatter round-trips the modifier.Known limits (tracked as follow-ups, documented in components.md/CHANGELOG)
state(theuse-store-introduces-fields design question) → [Components] Let a component reference a used store's fields without redeclaring state #355.clearinclient {}yet (runtime API exists) → [Client] Declarative store clear/reset in the client {} language #356.persistkeyword → [LSP] Editor support for the persist store modifier #357.Issue Closure
Related #352 (implements the recommended Option C). Follow-ups: #354, #355, #356, #357.
Verification
go test ./internal/... ./runtime/... ./cmd/...green — 37 pkgs;gofmt -lclean;go vetclean.)scripts/test-go-modules.shwhen Go code or compiler behavior changed. (EXIT=0, 0 failures.)go build ./cmd/gowdkwhen CLI, compiler, runtime, addon, or release behavior changed. (Also verifiedgowdk check/build/fmt/explainonexamples/store-persist.)node --check editors/vscode/extension.jswhen editor files changed. (N/A — no editor files changed; LSP support deferred to [LSP] Editor support for the persist store modifier #357.)docs/language/components.md,syntax.md, README row, CHANGELOG,examples/store-persist.)LLM Assistance