From 4d22a75e9c0927a6339f040cd7d0692947a85af0 Mon Sep 17 00:00:00 2001 From: Bruno Carvalho Date: Sat, 13 Jun 2026 11:43:55 -0300 Subject: [PATCH 1/5] feat(stores): client-side persistence for page stores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- CHANGELOG.md | 14 ++ README.md | 2 +- docs/language/components.md | 43 ++++- docs/language/syntax.md | 12 ++ examples/store-persist/README.md | 55 ++++++ examples/store-persist/add-button.cmp.gwdk | 21 +++ examples/store-persist/cart-badge.cmp.gwdk | 22 +++ examples/store-persist/shop.page.gwdk | 20 +++ examples/store-persist/ui/cart.go | 14 ++ internal/buildgen/render.go | 75 +++++++- internal/buildgen/runtime_assets.go | 151 ++++++++++++++-- .../buildgen/store_persist_browser_test.go | 135 ++++++++++++++ .../buildgen/store_persist_runtime_test.go | 168 ++++++++++++++++++ internal/buildgen/store_persist_test.go | 125 +++++++++++++ internal/clientrt/runtime.go | 6 + internal/compiler/validate.go | 1 + internal/compiler/validate_page.go | 52 +++++- .../compiler/validate_page_persist_test.go | 144 +++++++++++++++ internal/compiler/validate_stores.go | 61 +++++++ internal/diagnostics/explain.go | 37 ++++ internal/diagnostics/registry.go | 3 + internal/gwdkast/ast.go | 7 +- internal/gwdkir/ir.go | 5 +- .../conformance/accept/store_persist.gwdk | 12 ++ .../reject/store_persist_bad_scope.gwdk | 13 ++ internal/parser/page_lower.go | 9 +- internal/parser/page_test.go | 39 ++++ internal/parser/patterns.go | 15 +- internal/parser/syntax.go | 9 +- internal/syntax/declparse.go | 18 +- testfixture/islands/islands.go | 11 ++ 31 files changed, 1266 insertions(+), 33 deletions(-) create mode 100644 examples/store-persist/README.md create mode 100644 examples/store-persist/add-button.cmp.gwdk create mode 100644 examples/store-persist/cart-badge.cmp.gwdk create mode 100644 examples/store-persist/shop.page.gwdk create mode 100644 examples/store-persist/ui/cart.go create mode 100644 internal/buildgen/store_persist_browser_test.go create mode 100644 internal/buildgen/store_persist_runtime_test.go create mode 100644 internal/buildgen/store_persist_test.go create mode 100644 internal/compiler/validate_page_persist_test.go create mode 100644 internal/lang/testdata/conformance/accept/store_persist.gwdk create mode 100644 internal/lang/testdata/conformance/reject/store_persist_bad_scope.gwdk diff --git a/CHANGELOG.md b/CHANGELOG.md index cd5e4c2c..99ece59d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ packages, and tooling contracts may change before a stable release. ### Implemented +- Page stores can opt into browser persistence with a `persist "local"` or + `persist "session"` modifier + (`store cart ui.CartState = ui.NewCartState() persist "local"`). The generated + store runtime hydrates from localStorage/sessionStorage on load, re-hydrates on + SPA navigation (stores first declared on a later route are picked up on content + swap), writes the store's declared fields on change, mirrors cross-tab writes + through the `storage` event, exposes `window.__gowdkStores.clear(name)` to drop + a persisted store, and discards persisted state whose embedded schema hash no longer + matches the store's shape (so a struct change never restores stale data). + Only the store's own 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). Persistence is a JS-island/store + runtime feature; WASM islands do not yet participate in page stores. - M4 Go interop is complete for the current 0.x surface: a user can see why a Go function or type did or did not bind. `gowdk inspect go-bindings` emits a versioned JSON report (schema version 1) covering actions, APIs, fragments, diff --git a/README.md b/README.md index f736f2e8..9a01e451 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,7 @@ This table describes the current demoable 0.x slice. Status levels: | Fragments | Works, contract unstable | Partial form submissions and standalone fragment routes can return server fragments and remount local islands. | Richer fragment rendering and broader local client behavior are still hardening work. | [Partials](docs/language/partials.md) | [Fragments](examples/partials/patients-fragment.page.gwdk) | | SSR | Works, contract unstable | Pages with `load {}` or `go ssr {}` can build request-time handlers when the SSR addon is enabled. | Typed route-param accessors, lifecycle docs, and error/cache contracts need more hardening. | [SSR](docs/language/ssr.md) | [SSR](examples/ssr/simple-ssr.page.gwdk) | | Hybrid | Early | Hybrid request-time route metadata and generated request-time pages exist for the supported slice. | The public hybrid source contract, streaming, and data refresh policy are not stable. | [Hybrid](docs/language/hybrid.md) | [Hybrid](examples/ssr/hybrid-static.page.gwdk) | -| Components | Works, contract unstable | Components support imported contracts, slots, scoped CSS/assets, first local client behavior, and generated island assets. | Non-string props, richer slots/events, real `g:if`/`g:for`, lifecycle cleanup, and dependency diagnostics are planned. | [Components](docs/language/components.md) | [Components](examples/components/base/base-components.page.gwdk) | +| Components | Works, contract unstable | Components support imported contracts, slots, scoped CSS/assets, first local client behavior, and generated island assets. Page stores can opt into localStorage/sessionStorage persistence with `persist "local"`/`persist "session"`. | Non-string props, richer slots/events, real `g:if`/`g:for`, lifecycle cleanup, dependency diagnostics, and store persistence for WASM islands are planned. | [Components](docs/language/components.md) | [Components](examples/components/base/base-components.page.gwdk) | | WASM islands | Early | Component-level `wasm` and page-level `go client {}` can emit Go `js/wasm` browser assets for supported fixtures. | ABI docs, size reporting, runtime validation, and browser behavior coverage need hardening. | [Components](docs/language/components.md) | [Test fixture](testfixture/islands/islands.go) | | CSS/assets | Works, contract unstable | CSS processors, page CSS, scoped component CSS, component assets, asset manifests, content-hashed filenames, and optional Tailwind wrapper exist. | CSS processor contracts and optional dependency boundaries need hardening. | [CSS](docs/reference/css.md) | [CSS](examples/css/styled.page.gwdk) | | One-binary output | Works, contract unstable | `gowdk build --app --bin` can generate and compile an embedded Go server for supported SPA/backend/SSR slices. | Runtime operations, split/backend-only deploys, and artifact smoke coverage are still expanding. | [Deployment](docs/reference/deployment.md) | [Embed](examples/embed/site.page.gwdk) | diff --git a/docs/language/components.md b/docs/language/components.md index 79ca49cd..8c1d3f8b 100644 --- a/docs/language/components.md +++ b/docs/language/components.md @@ -275,7 +275,48 @@ Store use is explicit. Same-page stores use `client { use cart }`; stores from another discovered `.gwdk` package require a GOWDK `use` alias and a qualified client store reference such as `client { use stores.cart }`. Cross-package stores are validated by alias and store name, not discovered globally. -App-global stores and cross-route persistence are deferred. +App-global stores are deferred. + +A page store can opt into browser persistence with a `persist` modifier: + +```gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +store prefs ui.UIPrefs = ui.DefaultPrefs() persist "session" +``` + +`persist "local"` keeps the store in `localStorage` (survives a browser +restart); `persist "session"` keeps it in `sessionStorage` (survives reload and +SPA navigation, cleared when the tab closes). Persistence is keyed by store name +(`gowdk:store:`), so the same persisted store keeps its value across routes +on the origin. Only the store's own declared fields are persisted — never +component state, props, or computed values. The compiler embeds a hash of the +store's struct shape; when the shape changes, stale persisted state is discarded +rather than restored, so a struct change never crashes on old data. Because +browser storage is readable by any script on the origin, persisting a field whose +name resembles a secret (`token`, `password`, `secret`, `auth`, …) is a warning: +keep credentials and trusted authorization state server-side. An unknown scope is +rejected — see `gowdk explain page_store_persist_scope_invalid`. + +Persisted stores also sync across tabs: when one tab writes, other tabs on the +origin mirror the value through the browser `storage` event. To drop a persisted +store (for example after checkout or logout), call +`window.__gowdkStores.clear("")`, which removes the stored copy and resets +the store to its build-time init value. If two pages persist a store with the +same name but different shapes, they share one storage key and discard each +other's data on navigation; the compiler warns with +`page_store_persist_key_conflict`. + +Persistence survives SPA navigation: when the client runtime swaps page content +it re-scans store seeds, so a store first declared on a later client-side route +hydrates without a full page load, and a store already in memory keeps its value. + +Current limits. Persistence is a JS-island/store-runtime feature: WASM islands +do not yet participate in page stores, so a WASM-only island will not read or +write a persisted store. Because a component references store fields through its +own `state` declaration (`use ` syncs that field with the store), a +component that reads a persisted store still declares a matching `state` shape. +Invalid scopes are reported but not auto-fixed, because choosing `local` vs +`session` is a deliberate decision. Exports are typed component metadata today. They document values a component intends to expose, but parent pages/components do not yet have a stable runtime diff --git a/docs/language/syntax.md b/docs/language/syntax.md index ce7b3f62..684bacb4 100644 --- a/docs/language/syntax.md +++ b/docs/language/syntax.md @@ -489,6 +489,18 @@ or trusted authorization, validation, database, or action state. Runtime cross-island subscriptions are planned; the current contract validates declarations and explicit uses, but does not make stores global app state. +A page store may opt into browser persistence with a trailing `persist` scope: + +```gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +``` + +The scope must be `"local"` (localStorage) or `"session"` (sessionStorage); any +other value is rejected with `page_store_persist_scope_invalid`. Persisted store +state is keyed by store name, restores over the build-time init value on load, +is discarded when the store's struct shape changes, and warns when a persisted +field name resembles a secret. + Client blocks can declare limited DOM refs for safe methods: ```gwdk diff --git a/examples/store-persist/README.md b/examples/store-persist/README.md new file mode 100644 index 00000000..81162eeb --- /dev/null +++ b/examples/store-persist/README.md @@ -0,0 +1,55 @@ +# Persisted page store + +A page `store` that opts into browser persistence with `persist "local"`, so a +shared cart survives reloads and SPA navigation. + +```gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +``` + +- `ui/cart.go` — the user-owned Go state. GOWDK serializes only its declared + fields; it adds no opinions to the struct. +- `shop.page.gwdk` — declares the persisted store and renders the two islands. +- `add-button.cmp.gwdk` / `cart-badge.cmp.gwdk` — share the store via + `client { use cart }`. + +## Build it + +```sh +gowdk build --out ./dist \ + examples/store-persist/shop.page.gwdk \ + examples/store-persist/add-button.cmp.gwdk \ + examples/store-persist/cart-badge.cmp.gwdk +``` + +The generated `shop/index.html` carries the persist config on the store seed: + +```html + +``` + +and `assets/gowdk/islands/stores.js` hydrates the store from `localStorage` on +load and writes it back on every change. + +## What to try + +1. Open `/shop`, click **Add to cart** a few times — the badge counts up. +2. Reload the page. The badge keeps your count (restored from `localStorage`). +3. Change `CartState`'s shape in `ui/cart.go` and rebuild. The old persisted + value is discarded automatically (the embedded schema hash changes), so the + store falls back to its init value instead of restoring stale data. + +## Scopes + +- `persist "local"` — `localStorage`; survives a browser restart, shared across + all tabs and routes on the origin. +- `persist "session"` — `sessionStorage`; survives reload and SPA navigation, + cleared when the tab closes. + +Persisted store state is browser-visible: never persist secrets, session tokens, +or trusted authorization state. The compiler warns +(`page_store_persist_secret_field`) when a persisted field name resembles a +secret. diff --git a/examples/store-persist/add-button.cmp.gwdk b/examples/store-persist/add-button.cmp.gwdk new file mode 100644 index 00000000..9b6aea8d --- /dev/null +++ b/examples/store-persist/add-button.cmp.gwdk @@ -0,0 +1,21 @@ +package shop + +import ui "github.com/cssbruno/gowdk/examples/store-persist/ui" + +component AddButton + +// The local state shape matches the cart store; `use cart` syncs Count across +// every island that uses the store, and the page's persist "local" keeps it. +state ui.CartState = ui.NewCartState() + +client { + use cart + + func Add() { + Count++ + } +} + +view { + +} diff --git a/examples/store-persist/cart-badge.cmp.gwdk b/examples/store-persist/cart-badge.cmp.gwdk new file mode 100644 index 00000000..ff7e2e01 --- /dev/null +++ b/examples/store-persist/cart-badge.cmp.gwdk @@ -0,0 +1,22 @@ +package shop + +import ui "github.com/cssbruno/gowdk/examples/store-persist/ui" + +component CartBadge + +state ui.CartState = ui.NewCartState() + +client { + use cart + + computed Label string { + if Count == 0 { + return "Cart empty" + } + return string(Count) + } +} + +view { + 0}>{Label} +} diff --git a/examples/store-persist/shop.page.gwdk b/examples/store-persist/shop.page.gwdk new file mode 100644 index 00000000..ec181676 --- /dev/null +++ b/examples/store-persist/shop.page.gwdk @@ -0,0 +1,20 @@ +package shop + +import ui "github.com/cssbruno/gowdk/examples/store-persist/ui" + +page shop +route "/shop" +guard public + +// persist "local" keeps the cart in localStorage. Add to cart, reload the page, +// and the badge still shows your count. The same store is shared across routes +// on this origin and discarded automatically if CartState's shape changes. +store cart ui.CartState = ui.NewCartState() persist "local" + +view { +
+

Shop

+ + +
+} diff --git a/examples/store-persist/ui/cart.go b/examples/store-persist/ui/cart.go new file mode 100644 index 00000000..95d7c131 --- /dev/null +++ b/examples/store-persist/ui/cart.go @@ -0,0 +1,14 @@ +// Package ui holds the user-owned Go state for the store-persistence example. +// GOWDK never touches this struct; it only serializes its declared fields. +package ui + +// CartState is the cart store's Go type. When the page store opts into +// persist "local", only these declared fields are written to browser storage. +type CartState struct { + Count int `json:"count"` +} + +// NewCartState is the build-time initializer for the cart store. +func NewCartState() CartState { + return CartState{Count: 0} +} diff --git a/internal/buildgen/render.go b/internal/buildgen/render.go index 39c6d440..393613d9 100644 --- a/internal/buildgen/render.go +++ b/internal/buildgen/render.go @@ -1,8 +1,12 @@ package buildgen import ( + "encoding/json" "fmt" + "hash/fnv" "net/url" + "sort" + "strconv" "strings" "github.com/cssbruno/gowdk" @@ -284,8 +288,18 @@ func isInternalNavigationHref(value string) bool { } type pageStoreSeed struct { - Name string - JSON string + Name string + JSON string + Persist *storePersistSeed +} + +// storePersistSeed is the persistence config carried to the browser store +// registry on the seed ") + attrs := gowhtml.Attr("data-gowdk-store", seed.Name) + if seed.Persist != nil { + attrs += gowhtml.Attr("data-gowdk-persist", seed.Persist.Scope) + attrs += gowhtml.Attr("data-gowdk-persist-key", seed.Persist.Key) + attrs += gowhtml.Attr("data-gowdk-persist-version", seed.Persist.Version) + } + head = append(head, " ") } for _, script := range nonEmptyScripts(scripts) { tag := " { + const storageFor = (scope) => { + try { + return scope === "session" ? window.sessionStorage : window.localStorage; + } catch (error) { + return null; + } + }; + + const projectFields = (name, source) => { + const projected = Object.create(null); + (registry.fields[name] || []).forEach((field) => { + if (Object.prototype.hasOwnProperty.call(source, field)) projected[field] = source[field]; + }); + return projected; + }; + + const decodePersisted = (config, fields, raw) => { + if (!raw) return null; + let blob = null; + try { + blob = JSON.parse(raw); + } catch (error) { + return null; + } + if (!blob || blob.v !== config.version || typeof blob.s !== "object" || blob.s === null) return null; + const restored = Object.create(null); + fields.forEach((field) => { + if (Object.prototype.hasOwnProperty.call(blob.s, field)) restored[field] = blob.s[field]; + }); + return restored; + }; + + const readPersisted = (config, fields) => { + const storage = storageFor(config.scope); + if (!storage) return null; + let raw = null; + try { + raw = storage.getItem(config.key); + } catch (error) { + return null; + } + return decodePersisted(config, fields, raw); + }; + + const writePersisted = (name) => { + const config = registry.persist[name]; + if (!config) return; + const storage = storageFor(config.scope); + if (!storage) return; + try { + storage.setItem(config.key, JSON.stringify({ v: config.version, s: projectFields(name, registry.stores[name] || {}) })); + } catch (error) { + // Quota, private-mode, or disabled storage must never break the island. + if (!warned[name] && typeof console !== "undefined" && console.warn) { + warned[name] = true; + console.warn("GOWDK: could not persist store \"" + name + "\" (storage unavailable or full); continuing without persistence."); + } + } + }; + + const notify = (name) => { + (registry.listeners[name] || []).slice().forEach((listener) => listener(registry.get(name))); + }; + + registry.init = (name, state, persist) => { if (!name || registry.stores[name]) return; - registry.stores[name] = Object.assign({}, state || {}); + const seed = Object.assign(Object.create(null), state || {}); + registry.fields[name] = Object.keys(seed); + registry.seeds[name] = Object.assign(Object.create(null), seed); + if (persist && persist.scope && persist.key && persist.version) { + registry.persist[name] = persist; + const restored = readPersisted(persist, registry.fields[name]); + if (restored) Object.assign(seed, restored); + } + registry.stores[name] = seed; }; registry.get = (name) => { @@ -81,7 +157,25 @@ func storeRuntimeSource() string { registry.set = (name, next) => { if (!name) return; registry.stores[name] = Object.assign({}, registry.stores[name] || {}, next || {}); - (registry.listeners[name] || []).slice().forEach((listener) => listener(registry.get(name))); + writePersisted(name); + notify(name); + }; + + // clear drops the persisted copy and resets the in-memory store to its build + // -time seed, then notifies islands. Use after checkout, logout, or reset. + registry.clear = (name) => { + if (!name) return; + const config = registry.persist[name]; + if (config) { + const storage = storageFor(config.scope); + if (storage) { + try { + storage.removeItem(config.key); + } catch (error) {} + } + } + if (registry.seeds[name]) registry.stores[name] = Object.assign({}, registry.seeds[name]); + notify(name); }; registry.subscribe = (name, listener) => { @@ -93,14 +187,49 @@ func storeRuntimeSource() string { }; }; - document.querySelectorAll("script[type=\"application/json\"][data-gowdk-store]").forEach((node) => { - const name = node.getAttribute("data-gowdk-store"); - try { - registry.init(name, JSON.parse(node.textContent || "{}")); - } catch (error) { - registry.init(name, {}); - } - }); + // 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) => { + if (!event || !event.key) return; + Object.keys(registry.persist).forEach((name) => { + const config = registry.persist[name]; + if (!config || config.key !== event.key) return; + if (event.newValue == null) { + if (registry.seeds[name]) registry.stores[name] = Object.assign({}, registry.seeds[name]); + } else { + const restored = decodePersisted(config, registry.fields[name] || [], event.newValue); + if (restored) registry.stores[name] = Object.assign({}, registry.stores[name] || {}, restored); + } + notify(name); + }); + }); + } + + // hydrate scans the current document for store seeds and initializes any not + // already in the registry. It is idempotent (init bails on existing stores), + // so the SPA navigation runtime can call it after swapping page content to + // pick up stores first declared on a later route. + registry.hydrate = () => { + document.querySelectorAll("script[type=\"application/json\"][data-gowdk-store]").forEach((node) => { + const name = node.getAttribute("data-gowdk-store"); + let persist = null; + const scope = node.getAttribute("data-gowdk-persist"); + if (scope) { + persist = { + scope: scope, + key: node.getAttribute("data-gowdk-persist-key") || ("gowdk:store:" + name), + version: node.getAttribute("data-gowdk-persist-version") || "" + }; + } + try { + registry.init(name, JSON.parse(node.textContent || "{}"), persist); + } catch (error) { + registry.init(name, {}, persist); + } + }); + }; + registry.hydrate(); })(); `) } diff --git a/internal/buildgen/store_persist_browser_test.go b/internal/buildgen/store_persist_browser_test.go new file mode 100644 index 00000000..aedfdeb8 --- /dev/null +++ b/internal/buildgen/store_persist_browser_test.go @@ -0,0 +1,135 @@ +package buildgen + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/cssbruno/gowdk" + "github.com/cssbruno/gowdk/internal/gwdkanalysis" + "github.com/cssbruno/gowdk/internal/gwdkir" +) + +// TestPersistedStoreSurvivesReloadInBrowser builds a page whose store opts into +// persist "local", drives a real browser to mutate it, reloads, and asserts the +// value is restored from localStorage. Skips unless node + chromium + playwright +// are available (same gating as the other browser harness tests). +func TestPersistedStoreSurvivesReloadInBrowser(t *testing.T) { + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is not installed") + } + chromium, err := lookupChromium() + if err != nil { + t.Skip(err) + } + requireNodePlaywright(t, node) + + outputDir := t.TempDir() + component := counterComponent() + component.Blocks.Client = true + component.Blocks.ClientBody = `use cart` + app := gwdkanalysis.Sources{ + Pages: []gwdkir.Page{{ + ID: "counter", + Route: "/counter", + Imports: []gwdkir.Import{{Alias: "ui", Path: "github.com/cssbruno/gowdk/testfixture/islands"}}, + Stores: []gwdkir.Store{{ + Name: "cart", + Type: gwdkir.GoRef{Alias: "ui", Name: "CounterState"}, + Init: gwdkir.GoRef{Alias: "ui", Name: "NewCounterState"}, + Persist: "local", + }}, + Blocks: gwdkir.Blocks{View: true, ViewBody: `
`}, + }}, + Components: []gwdkir.Component{component}, + } + if _, err := Build(gowdk.Config{}, app, outputDir); err != nil { + t.Fatal(err) + } + + server := httptest.NewServer(http.FileServer(http.Dir(outputDir))) + defer server.Close() + + script := filepath.Join(t.TempDir(), "gowdk-persist-browser-test.cjs") + if err := os.WriteFile(script, []byte(persistedStoreBrowserHarness()), 0o600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + command := exec.CommandContext(ctx, node, script, server.URL, chromium) + command.Dir = mustWorkingDir(t) + output, err := command.CombinedOutput() + if ctx.Err() != nil { + t.Fatalf("persisted store browser test timed out:\n%s", output) + } + if err != nil { + t.Fatalf("persisted store browser test failed: %v\n%s", err, output) + } +} + +func persistedStoreBrowserHarness() string { + return ` +"use strict"; + +const assert = require("node:assert/strict"); +const nodeModule = require("node:module"); + +const baseURL = process.argv[2]; +const executablePath = process.argv[3]; +const { chromium } = nodeModule.createRequire(process.cwd() + "/gowdk-test.js")("playwright"); + +async function waitForButtonText(page, expected) { + await page.waitForFunction((expected) => { + return document.querySelector("gowdk-island button")?.textContent === expected; + }, expected); +} + +(async () => { + const browser = await chromium.launch({ executablePath, headless: true, args: ["--no-sandbox"] }); + const context = await browser.newContext(); + const page = await context.newPage(); + const consoleErrors = []; + page.on("console", (message) => { + if (message.type() === "error" && message.text().includes("GOWDK")) consoleErrors.push(message.text()); + }); + + // Initial load: seed is Count 1, nothing persisted yet. + await page.goto(baseURL + "/counter/", { waitUntil: "networkidle" }); + await waitForButtonText(page, "1"); + + // Mutate to 3; each click syncs to the persisted cart store. + await page.evaluate(() => document.querySelector("gowdk-island button").click()); + await page.evaluate(() => document.querySelector("gowdk-island button").click()); + await waitForButtonText(page, "3"); + + // localStorage carries the versioned, field-projected value. + const stored = await page.evaluate(() => window.localStorage.getItem("gowdk:store:cart")); + const blob = JSON.parse(stored); + assert.equal(blob.s.Count, 3, "store value persisted to localStorage"); + assert.ok(typeof blob.v === "string" && blob.v.length > 0, "persisted blob carries a version"); + + // Reload: the island must hydrate Count 3 from localStorage, not the seed. + await page.goto(baseURL + "/counter/", { waitUntil: "networkidle" }); + await waitForButtonText(page, "3"); + assert.equal(await page.evaluate(() => window.__gowdkStores.get("cart").Count), 3, "store rehydrated after reload"); + + // clear() drops the persisted copy; a fresh reload returns to the seed. + await page.evaluate(() => window.__gowdkStores.clear("cart")); + assert.equal(await page.evaluate(() => window.localStorage.getItem("gowdk:store:cart")), null, "clear removed storage"); + await page.goto(baseURL + "/counter/", { waitUntil: "networkidle" }); + await waitForButtonText(page, "1"); + + assert.deepEqual(consoleErrors, []); + await browser.close(); +})().catch(async (error) => { + console.error(error && error.stack || error); + process.exit(1); +}); +` +} diff --git a/internal/buildgen/store_persist_runtime_test.go b/internal/buildgen/store_persist_runtime_test.go new file mode 100644 index 00000000..de542d19 --- /dev/null +++ b/internal/buildgen/store_persist_runtime_test.go @@ -0,0 +1,168 @@ +package buildgen + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// TestStoreRuntimePersistenceUnderNode executes the generated store runtime in +// node against mocked window/localStorage and asserts the real behavior of the +// persistence path: hydrate-on-load, field projection, version invalidation, +// clear, quota-failure tolerance, and cross-tab storage-event sync. It needs +// only node (no chromium/playwright), so it runs in more environments than the +// full browser harness. +func TestStoreRuntimePersistenceUnderNode(t *testing.T) { + node, err := exec.LookPath("node") + if err != nil { + t.Skip("node is not installed") + } + + dir := t.TempDir() + runtimePath := filepath.Join(dir, "stores.js") + if err := os.WriteFile(runtimePath, []byte(storeRuntimeSource()), 0o600); err != nil { + t.Fatal(err) + } + harnessPath := filepath.Join(dir, "harness.cjs") + if err := os.WriteFile(harnessPath, []byte(storeRuntimeNodeHarness()), 0o600); err != nil { + t.Fatal(err) + } + + output, err := exec.Command(node, harnessPath, runtimePath).CombinedOutput() + if err != nil { + t.Fatalf("store runtime behavior test failed: %v\n%s", err, output) + } +} + +func storeRuntimeNodeHarness() string { + return ` +"use strict"; +const assert = require("node:assert/strict"); +const fs = require("node:fs"); + +const runtimeSrc = fs.readFileSync(process.argv[2], "utf8"); + +function makeStorage() { + const map = new Map(); + return { + failNext: false, + getItem(k) { return map.has(k) ? map.get(k) : null; }, + setItem(k, v) { if (this.failNext) { this.failNext = false; throw new Error("QuotaExceededError"); } map.set(k, String(v)); }, + removeItem(k) { map.delete(k); } + }; +} + +const localStorage = makeStorage(); +const sessionStorage = makeStorage(); +let storageListeners = []; +let warnings = []; + +// Harness-controlled seed and persist attributes, read fresh on every boot. +let seedJSON = '{"Count":0,"Open":false}'; +let scope = "local"; +let version = "v1"; + +function makeNode() { + return { + getAttribute(name) { + if (name === "data-gowdk-store") return "cart"; + if (name === "data-gowdk-persist") return scope; + if (name === "data-gowdk-persist-key") return "gowdk:store:cart"; + if (name === "data-gowdk-persist-version") return version; + return null; + }, + get textContent() { return seedJSON; } + }; +} + +global.console = { warn: (m) => warnings.push(m), error: (m) => warnings.push(m), log: console.log }; +global.document = { querySelectorAll: () => [makeNode()] }; + +function boot() { + // Fresh registry each boot simulates a page load; storage persists across boots. + global.window = { + localStorage, + sessionStorage, + addEventListener(type, fn) { if (type === "storage") storageListeners.push(fn); } + }; + storageListeners = []; + new Function(runtimeSrc)(); + return global.window.__gowdkStores; +} + +// 1. Empty storage -> seed. +let r = boot(); +assert.deepEqual(r.get("cart"), { Count: 0, Open: false }, "fresh load should equal seed"); + +// 2. set persists only declared fields (Extra is dropped). +r.set("cart", { Count: 5, Open: true, Extra: "x" }); +const stored = JSON.parse(localStorage.getItem("gowdk:store:cart")); +assert.equal(stored.v, "v1", "stored version"); +assert.deepEqual(stored.s, { Count: 5, Open: true }, "only seed fields persist, not Extra"); + +// 3. Reload hydrates from storage. +r = boot(); +assert.equal(r.get("cart").Count, 5, "reload should restore persisted Count"); + +// 4. Version change discards stale storage. +version = "v2"; +r = boot(); +assert.equal(r.get("cart").Count, 0, "shape/version change should discard stale data"); + +// 5. clear() removes storage and resets to seed, notifying subscribers. +version = "v1"; +r = boot(); +r.set("cart", { Count: 9, Open: false }); +r = boot(); +assert.equal(r.get("cart").Count, 9, "precondition: persisted 9"); +let notified = null; +r.subscribe("cart", (next) => { notified = next; }); +r.clear("cart"); +assert.equal(localStorage.getItem("gowdk:store:cart"), null, "clear removes the storage key"); +assert.equal(r.get("cart").Count, 0, "clear resets to seed"); +assert.equal(notified.Count, 0, "clear notifies subscribers"); + +// 6. Quota failure on write must not throw and must warn once. +r = boot(); +warnings = []; +localStorage.failNext = true; +assert.doesNotThrow(() => r.set("cart", { Count: 3 }), "write failure must not throw"); +assert.equal(r.get("cart").Count, 3, "in-memory state still updates on write failure"); +assert.ok(warnings.some((m) => m.includes("GOWDK")), "a one-time GOWDK warning is logged"); + +// 7. Cross-tab storage event mirrors the value and notifies, without writing back. +r = boot(); +let crossTab = null; +r.subscribe("cart", (next) => { crossTab = next; }); +assert.ok(storageListeners.length > 0, "a storage listener is registered"); +storageListeners[0]({ key: "gowdk:store:cart", newValue: JSON.stringify({ v: "v1", s: { Count: 42, Open: false } }) }); +assert.equal(r.get("cart").Count, 42, "cross-tab write is mirrored"); +assert.equal(crossTab.Count, 42, "cross-tab write notifies subscribers"); +// A cleared key in another tab resets to seed. +storageListeners[0]({ key: "gowdk:store:cart", newValue: null }); +assert.equal(r.get("cart").Count, 0, "cross-tab clear resets to seed"); + +// 8. SPA-navigation re-hydration: a store first declared on a later route is +// picked up by hydrate() without re-running the runtime, and existing stores +// are left untouched (init bails on stores already in the registry). +global.document.querySelectorAll = () => [ + makeNode(), + { + getAttribute(name) { + if (name === "data-gowdk-store") return "prefs"; + if (name === "data-gowdk-persist") return "local"; + if (name === "data-gowdk-persist-key") return "gowdk:store:prefs"; + if (name === "data-gowdk-persist-version") return "p1"; + return null; + }, + get textContent() { return '{"Theme":"dark"}'; } + } +]; +r.hydrate(); +assert.equal(r.get("prefs").Theme, "dark", "hydrate() picks up a store first seen on a later route"); +assert.equal(r.get("cart").Count, 0, "re-hydrate leaves an existing store untouched"); + +console.log("OK"); +` +} diff --git a/internal/buildgen/store_persist_test.go b/internal/buildgen/store_persist_test.go new file mode 100644 index 00000000..59fa10a8 --- /dev/null +++ b/internal/buildgen/store_persist_test.go @@ -0,0 +1,125 @@ +package buildgen + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/cssbruno/gowdk" + "github.com/cssbruno/gowdk/internal/gotypes" + "github.com/cssbruno/gowdk/internal/gwdkanalysis" + "github.com/cssbruno/gowdk/internal/gwdkir" +) + +func TestBuildEmitsPersistConfigOnStoreSeed(t *testing.T) { + outputDir := t.TempDir() + app := gwdkanalysis.Sources{ + Pages: []gwdkir.Page{{ + ID: "cart", + Route: "/cart", + Imports: []gwdkir.Import{{Alias: "ui", Path: "github.com/cssbruno/gowdk/testfixture/islands"}}, + Stores: []gwdkir.Store{{ + Name: "cart", + Type: gwdkir.GoRef{Alias: "ui", Name: "CounterState"}, + Init: gwdkir.GoRef{Alias: "ui", Name: "NewCounterState"}, + Persist: "local", + }}, + Blocks: gwdkir.Blocks{View: true, ViewBody: `
Cart
`}, + }}, + } + + result, err := Build(gowdk.Config{}, app, outputDir) + if err != nil { + t.Fatal(err) + } + storePath := filepath.Join(outputDir, "assets", "gowdk", "islands", "stores.js") + if !hasAssetArtifact(result.AssetArtifacts, storePath) { + t.Fatalf("expected stores.js asset, got %#v", result.AssetArtifacts) + } + + html := readFile(t, filepath.Join(outputDir, "cart", "index.html")) + for _, expected := range []string{ + `data-gowdk-store="cart"`, + `data-gowdk-persist="local"`, + `data-gowdk-persist-key="gowdk:store:cart"`, + `data-gowdk-persist-version="`, + } { + if !strings.Contains(html, expected) { + t.Fatalf("expected %q in persisted store page:\n%s", expected, html) + } + } + // The version attribute must carry a non-empty hash. + if strings.Contains(html, `data-gowdk-persist-version=""`) { + t.Fatalf("persist version attribute is empty:\n%s", html) + } + + storeJS := readFile(t, storePath) + for _, expected := range []string{ + "registry.persist", + "readPersisted", + "writePersisted", + "window.localStorage", + "window.sessionStorage", + "data-gowdk-persist", + } { + if !strings.Contains(storeJS, expected) { + t.Fatalf("expected %q in store runtime:\n%s", expected, storeJS) + } + } +} + +func TestBuildOmitsPersistConfigForUnpersistedStore(t *testing.T) { + outputDir := t.TempDir() + app := gwdkanalysis.Sources{ + Pages: []gwdkir.Page{{ + ID: "cart", + Route: "/cart", + Imports: []gwdkir.Import{{Alias: "ui", Path: "github.com/cssbruno/gowdk/testfixture/islands"}}, + Stores: []gwdkir.Store{{ + Name: "cart", + Type: gwdkir.GoRef{Alias: "ui", Name: "CounterState"}, + Init: gwdkir.GoRef{Alias: "ui", Name: "NewCounterState"}, + }}, + Blocks: gwdkir.Blocks{View: true, ViewBody: `
Cart
`}, + }}, + } + + if _, err := Build(gowdk.Config{}, app, outputDir); err != nil { + t.Fatal(err) + } + html := readFile(t, filepath.Join(outputDir, "cart", "index.html")) + if strings.Contains(html, "data-gowdk-persist") { + t.Fatalf("did not expect persist attributes on an unpersisted store:\n%s", html) + } +} + +func TestStoreSchemaHashIsStableAndShapeSensitive(t *testing.T) { + base := gotypes.Struct{FieldTypes: map[string]string{"Count": "int", "Open": "bool"}} + baseSeed := `{"count":0,"open":false}` + // Same shape, different map iteration order must produce the same hash. + same := gotypes.Struct{FieldTypes: map[string]string{"Open": "bool", "Count": "int"}} + if storeSchemaHash(base, baseSeed) != storeSchemaHash(same, baseSeed) { + t.Fatalf("schema hash should be order-independent: %q vs %q", storeSchemaHash(base, baseSeed), storeSchemaHash(same, baseSeed)) + } + if storeSchemaHash(base, baseSeed) == "" { + t.Fatal("schema hash should be non-empty") + } + // A retyped field must change the hash (stale storage would otherwise restore a wrong-typed value). + retyped := gotypes.Struct{FieldTypes: map[string]string{"Count": "string", "Open": "bool"}} + if storeSchemaHash(base, baseSeed) == storeSchemaHash(retyped, baseSeed) { + t.Fatal("retyping a field should change the schema hash") + } + // A removed field must change the hash. + removed := gotypes.Struct{FieldTypes: map[string]string{"Count": "int"}} + if storeSchemaHash(base, `{"count":0}`) == storeSchemaHash(base, baseSeed) { + t.Fatal("removing a field (fewer on-wire keys) should change the schema hash") + } + if storeSchemaHash(base, baseSeed) == storeSchemaHash(removed, `{"count":0}`) { + t.Fatal("removing a field should change the schema hash") + } + // A json-tag-only rename (same Go fields, different on-wire key) must change the hash. + renamedSeed := `{"qty":0,"open":false}` + if storeSchemaHash(base, baseSeed) == storeSchemaHash(base, renamedSeed) { + t.Fatal("a json-tag-only rename should change the schema hash") + } +} diff --git a/internal/clientrt/runtime.go b/internal/clientrt/runtime.go index fd79984a..62558457 100644 --- a/internal/clientrt/runtime.go +++ b/internal/clientrt/runtime.go @@ -60,6 +60,9 @@ const runtimeSource = `(function () { } else { target.innerHTML = html; } + if (typeof window !== 'undefined' && window.__gowdkStores && window.__gowdkStores.hydrate) { + window.__gowdkStores.hydrate(); + } if (typeof window !== 'undefined' && window.__gowdkMountIslands) { window.__gowdkMountIslands(); } @@ -190,6 +193,9 @@ const runtimeSource = `(function () { if (push && window.history && window.history.pushState) { window.history.pushState({}, document.title, url); } + if (typeof window !== 'undefined' && window.__gowdkStores && window.__gowdkStores.hydrate) { + window.__gowdkStores.hydrate(); + } if (typeof window !== 'undefined' && window.__gowdkMountIslands) { window.__gowdkMountIslands(); } diff --git a/internal/compiler/validate.go b/internal/compiler/validate.go index 4613e752..1ca5e384 100644 --- a/internal/compiler/validate.go +++ b/internal/compiler/validate.go @@ -88,6 +88,7 @@ func validateProgram(config gowdk.Config, ir gwdkir.Program, crossFile bool) Val diagnostics = append(diagnostics, validateComponentEmits(ir.Components)...) diagnostics = append(diagnostics, validateComponentGoContracts(ir.Components)...) diagnostics = append(diagnostics, validateComponentStoreUses(ir.Pages, ir.Components)...) + diagnostics = append(diagnostics, validatePersistedStoreConflicts(ir.Pages)...) diagnostics = append(diagnostics, validateRedundantComponents(ir.Components)...) diagnostics = append(diagnostics, validateGOWDKUses(ir, crossFile)...) diagnostics = append(diagnostics, validatePageAssetUses(ir)...) diff --git a/internal/compiler/validate_page.go b/internal/compiler/validate_page.go index 47e306ad..151393be 100644 --- a/internal/compiler/validate_page.go +++ b/internal/compiler/validate_page.go @@ -376,7 +376,8 @@ func validatePageStores(page gwdkir.Page) []ValidationError { continue } seen[store.Name] = store - if _, err := gotypes.ResolveStruct(page.Imports, store.Type); err != nil { + resolved, err := gotypes.ResolveStruct(page.Imports, store.Type) + if err != nil { diagnostics = append(diagnostics, ValidationError{ Code: "page_store_error", PageID: page.ID, @@ -395,10 +396,59 @@ func validatePageStores(page gwdkir.Page) []ValidationError { Message: fmt.Sprintf("page %s store %q init is invalid: %v", page.ID, store.Name, err), }) } + diagnostics = append(diagnostics, validateStorePersist(page, store, resolved)...) } return diagnostics } +// validateStorePersist checks the optional `persist ""` modifier on a +// page store: the scope must be a known browser storage backend, and persisting +// a field whose name resembles a secret earns a warning because browser storage +// is readable by any script on the origin. +func validateStorePersist(page gwdkir.Page, store gwdkir.Store, resolved gotypes.Struct) []ValidationError { + if store.Persist == "" { + return nil + } + if store.Persist != "local" && store.Persist != "session" { + return []ValidationError{{ + Code: "page_store_persist_scope_invalid", + PageID: page.ID, + Source: page.Source, + Span: firstSpan(store.Span, page.Spans.Page), + Message: fmt.Sprintf("page %s store %q persist scope %q is invalid; use \"local\" or \"session\"", page.ID, store.Name, store.Persist), + }} + } + var diagnostics []ValidationError + for _, field := range resolved.Fields { + if !looksLikeSecretFieldName(field.Name) { + continue + } + diagnostics = append(diagnostics, ValidationError{ + Code: "page_store_persist_secret_field", + PageID: page.ID, + Source: page.Source, + Span: firstSpan(store.Span, page.Spans.Page), + Severity: SeverityWarning, + Message: fmt.Sprintf("page %s store %q persists field %q, which resembles a secret; %s browser storage is readable by any script on this origin", page.ID, store.Name, field.Name, store.Persist), + }) + } + return diagnostics +} + +// looksLikeSecretFieldName flags field names that commonly hold credentials or +// trusted authorization state, which the store contract already forbids from +// browser-visible state and which persistence would write to disk. +func looksLikeSecretFieldName(name string) bool { + lower := strings.ToLower(name) + needles := []string{"password", "passwd", "secret", "token", "apikey", "api_key", "auth", "credential", "private_key", "privatekey", "ssn"} + for _, needle := range needles { + if strings.Contains(lower, needle) { + return true + } + } + return false +} + func validatePageCSS(page gwdkir.Page) []ValidationError { if len(page.CSS) == 0 { return nil diff --git a/internal/compiler/validate_page_persist_test.go b/internal/compiler/validate_page_persist_test.go new file mode 100644 index 00000000..8ac089c4 --- /dev/null +++ b/internal/compiler/validate_page_persist_test.go @@ -0,0 +1,144 @@ +package compiler + +import ( + "testing" + + "github.com/cssbruno/gowdk" + "github.com/cssbruno/gowdk/internal/gwdkir" +) + +func persistedStorePageNamed(id, route, storeName, typeName, initName, scope string) gwdkir.Page { + return gwdkir.Page{ + ID: id, + Route: route, + Source: "pages/" + id + ".page.gwdk", + Guards: []string{"public"}, + Imports: []gwdkir.Import{{ + Alias: "ui", + Path: "github.com/cssbruno/gowdk/testfixture/islands", + }}, + Stores: []gwdkir.Store{{ + Name: storeName, + Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, + Init: gwdkir.GoRef{Alias: "ui", Name: initName}, + Persist: scope, + }}, + Blocks: gwdkir.Blocks{View: true, ViewBody: `
Page
`}, + } +} + +func persistedStorePage(storeName, typeName, initName, scope string) gwdkir.Page { + return gwdkir.Page{ + ID: "cart", + Route: "/cart", + Source: "pages/cart.page.gwdk", + Guards: []string{"public"}, + Imports: []gwdkir.Import{{ + Alias: "ui", + Path: "github.com/cssbruno/gowdk/testfixture/islands", + }}, + Stores: []gwdkir.Store{{ + Name: storeName, + Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, + Init: gwdkir.GoRef{Alias: "ui", Name: initName}, + Persist: scope, + }}, + Blocks: gwdkir.Blocks{View: true, ViewBody: `
Cart
`}, + } +} + +func TestValidatePageAcceptsPersistedStore(t *testing.T) { + for _, scope := range []string{"local", "session"} { + t.Run(scope, func(t *testing.T) { + page := persistedStorePage("cart", "CounterState", "NewCounterState", scope) + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + if ValidationErrors(diagnostics).HasErrors() { + t.Fatalf("persist %q should be valid, got %#v", scope, diagnostics) + } + }) + } +} + +func TestValidatePageRejectsInvalidPersistScope(t *testing.T) { + page := persistedStorePage("cart", "CounterState", "NewCounterState", "disk") + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + diagnostic := firstDiagnostic(diagnostics, "page_store_persist_scope_invalid") + if diagnostic == nil { + t.Fatalf("missing page_store_persist_scope_invalid diagnostic: %#v", diagnostics) + } + if diagnostic.Severity != SeverityError { + t.Fatalf("invalid persist scope should be an error, got severity %v", diagnostic.Severity) + } +} + +func TestValidatePageWarnsOnPersistedSecretField(t *testing.T) { + // SessionState has a Token field, which resembles a secret. + page := persistedStorePage("session", "SessionState", "NewSessionState", "local") + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + diagnostic := firstDiagnostic(diagnostics, "page_store_persist_secret_field") + if diagnostic == nil { + t.Fatalf("missing page_store_persist_secret_field diagnostic: %#v", diagnostics) + } + if diagnostic.Severity != SeverityWarning { + t.Fatalf("secret field should be a warning, got severity %v", diagnostic.Severity) + } + // A resembling-secret field name must not fail the build. + if ValidationErrors(diagnostics).HasErrors() { + t.Fatalf("secret field warning should not fail the build: %#v", diagnostics) + } +} + +func TestValidatePageDoesNotPersistCheckUnpersistedStore(t *testing.T) { + // A non-persisted store with a secret-looking field must stay silent. + page := persistedStorePage("session", "SessionState", "NewSessionState", "") + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + if d := firstDiagnostic(diagnostics, "page_store_persist_secret_field"); d != nil { + t.Fatalf("unexpected secret-field warning on a non-persisted store: %#v", d) + } +} + +func TestValidateWarnsOnPersistedStoreKeyConflict(t *testing.T) { + // Same store name, different shapes, both persisted -> shared key, divergent hash. + app := appFixture{Pages: []gwdkir.Page{ + persistedStorePageNamed("shop", "/shop", "cart", "CounterState", "NewCounterState", "local"), + persistedStorePageNamed("notes", "/notes", "cart", "TextState", "NewTextState", "local"), + }} + diagnostics := ValidateProgramReport(gowdk.Config{}, app.program(gowdk.Config{})) + d := firstDiagnostic(diagnostics, "page_store_persist_key_conflict") + if d == nil { + t.Fatalf("missing page_store_persist_key_conflict: %#v", diagnostics) + } + if d.Severity != SeverityWarning { + t.Fatalf("key conflict should be a warning, got %v", d.Severity) + } + if ValidationErrors(diagnostics).HasErrors() { + t.Fatalf("key conflict alone must not fail the build: %#v", diagnostics) + } +} + +func TestValidateAllowsSharedPersistedStoreAcrossPages(t *testing.T) { + // Same name AND same shape across pages is intentional cross-route sharing. + app := appFixture{Pages: []gwdkir.Page{ + persistedStorePageNamed("shop", "/shop", "cart", "CounterState", "NewCounterState", "local"), + persistedStorePageNamed("checkout", "/checkout", "cart", "CounterState", "NewCounterState", "local"), + }} + diagnostics := ValidateProgramReport(gowdk.Config{}, app.program(gowdk.Config{})) + if d := firstDiagnostic(diagnostics, "page_store_persist_key_conflict"); d != nil { + t.Fatalf("same-shape shared persisted store must not conflict: %#v", d) + } +} + +func TestLooksLikeSecretFieldName(t *testing.T) { + secrets := []string{"Token", "Password", "APIKey", "api_key", "Secret", "Credential", "Authenticated", "SSN", "PrivateKey"} + for _, name := range secrets { + if !looksLikeSecretFieldName(name) { + t.Errorf("expected %q to be flagged as secret-like", name) + } + } + safe := []string{"Count", "Open", "Query", "Items", "Theme", "Density"} + for _, name := range safe { + if looksLikeSecretFieldName(name) { + t.Errorf("did not expect %q to be flagged as secret-like", name) + } + } +} diff --git a/internal/compiler/validate_stores.go b/internal/compiler/validate_stores.go index 6ef14241..6b0172eb 100644 --- a/internal/compiler/validate_stores.go +++ b/internal/compiler/validate_stores.go @@ -2,12 +2,73 @@ package compiler import ( "fmt" + "sort" "strings" "github.com/cssbruno/gowdk/internal/clientlang" + "github.com/cssbruno/gowdk/internal/gotypes" "github.com/cssbruno/gowdk/internal/gwdkir" ) +// validatePersistedStoreConflicts warns when two pages persist a store under +// the same name but with different struct shapes. Persistence keys on the store +// name, so they share one browser storage slot; their differing schema hashes +// then discard each other's saved value on every navigation between the pages. +func validatePersistedStoreConflicts(pages []gwdkir.Page) []ValidationError { + type seenStore struct { + page gwdkir.Page + store gwdkir.Store + signature string + } + seen := map[string]seenStore{} + var diagnostics []ValidationError + for _, page := range pages { + for _, store := range page.Stores { + if store.Name == "" || (store.Persist != "local" && store.Persist != "session") { + continue + } + signature := persistedStoreSignature(page, store) + if signature == "" { + continue // unresolvable types are already reported as page_store_error + } + prior, exists := seen[store.Name] + if !exists { + seen[store.Name] = seenStore{page: page, store: store, signature: signature} + continue + } + if prior.signature == signature { + continue // same shape sharing one key across routes is intended + } + diagnostics = append(diagnostics, ValidationError{ + Code: "page_store_persist_key_conflict", + PageID: page.ID, + Source: page.Source, + Span: firstSpan(store.Span, page.Spans.Page), + Severity: SeverityWarning, + Related: relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted here on page %s", prior.store.Name, prior.page.ID)), + Message: fmt.Sprintf( + "persisted store %q has different shapes on pages %s and %s but shares browser storage key %q; navigating between them discards each other's saved data. Rename one store or give them matching shapes", + store.Name, prior.page.ID, page.ID, "gowdk:store:"+store.Name, + ), + }) + } + } + return diagnostics +} + +func persistedStoreSignature(page gwdkir.Page, store gwdkir.Store) string { + resolved, err := gotypes.ResolveStruct(page.Imports, store.Type) + if err != nil { + return "" + } + parts := make([]string, 0, len(resolved.Fields)) + for _, field := range resolved.Fields { + parts = append(parts, field.Name+":"+field.Type) + } + sort.Strings(parts) + return strings.Join(parts, ",") +} + func validateComponentStoreUses(pages []gwdkir.Page, components []gwdkir.Component) []ValidationError { declared := declaredStoreNamesByPackage(pages) if len(declared) == 0 { diff --git a/internal/diagnostics/explain.go b/internal/diagnostics/explain.go index b19b12c1..e1956328 100644 --- a/internal/diagnostics/explain.go +++ b/internal/diagnostics/explain.go @@ -27,6 +27,43 @@ type explanationDetail struct { } var explanationDetails = map[string]explanationDetail{ + "page_store_persist_key_conflict": { + Details: "Two pages declare a persisted store with the same name but different struct shapes. Persistence is keyed by store name (gowdk:store:), so both pages read and write the same browser storage slot. Because their embedded schema hashes differ, navigating from one page to the other discards the saved value every time. Either rename one store so each owns its own key, or give them the same shape so sharing is intentional.", + NextSteps: []string{ + "Rename one of the stores so each persisted store has a unique name.", + "Give both stores the same Go type when sharing one persisted value across pages is intended.", + }, + Invalid: `// pages/shop.page.gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +// pages/admin.page.gwdk (different shape, same name) +store cart admin.AuditState = admin.NewAuditState() persist "local"`, + Fixed: `// pages/shop.page.gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +// pages/admin.page.gwdk +store audit admin.AuditState = admin.NewAuditState() persist "local"`, + }, + "page_store_persist_scope_invalid": { + Details: "A page store may opt into browser persistence with persist \"local\" or persist \"session\". local uses window.localStorage (survives a browser restart); session uses window.sessionStorage (survives reload and SPA navigation, cleared when the tab closes). No other scope is supported.", + NextSteps: []string{ + "Use persist \"local\" to keep the store across browser restarts.", + "Use persist \"session\" to keep the store for the life of the tab.", + "Remove the persist modifier when the store should reset on reload.", + }, + Invalid: `store cart ui.CartState = ui.NewCartState() persist "disk"`, + Fixed: `store cart ui.CartState = ui.NewCartState() persist "local"`, + }, + "page_store_persist_secret_field": { + Details: "A persisted store field name resembles a secret (for example token, password, secret, or auth). Persisted store state is written to browser storage, which is readable by any script on the same origin, so it must never hold credentials, session tokens, or trusted authorization state. This is a warning, not an error: rename the field if it is not actually a secret.", + NextSteps: []string{ + "Keep secrets, session tokens, and authorization state in server-owned Go and never in a persisted store.", + "Rename the field if its name only resembles a secret but holds plain UI state.", + "Drop the persist modifier if the store legitimately needs sensitive values in memory but not on disk.", + }, + Invalid: `store session ui.SessionState = ui.NewSession() persist "local" +// SessionState has a Token field`, + Fixed: `store prefs ui.UIPrefs = ui.DefaultPrefs() persist "local" +// UIPrefs holds only non-sensitive UI state`, + }, "guard_requires_request_render": { Details: "Protected page guards must gate the page GET route at request time. A build-time SPA page emits plain static HTML, so it cannot enforce frontend page access.", NextSteps: []string{ diff --git a/internal/diagnostics/registry.go b/internal/diagnostics/registry.go index ed551822..b3d35183 100644 --- a/internal/diagnostics/registry.go +++ b/internal/diagnostics/registry.go @@ -130,6 +130,9 @@ var Registry = []Code{ {Code: "package_mismatch", Area: "packages", Stability: StabilityStable, Severity: SeverityError, Summary: "GOWDK package does not match sibling Go files"}, {Code: "package_must_be_first", Area: "packages", Stability: StabilityStable, Severity: SeverityError, Summary: "package declaration is not the first non-comment declaration"}, {Code: "page_store_error", Area: "stores", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page store type or initializer is invalid"}, + {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_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"}, {Code: "parse_error", Area: "parser", Stability: StabilityStable, Severity: SeverityError, Summary: "parser rejected source without a more specific code"}, {Code: "public_guard_exclusive", Area: "pages", Stability: StabilityStable, Severity: SeverityError, Summary: "guard public must be the only guard on an intentionally public page"}, {Code: "redundant_component_implementation", Area: "components", Stability: StabilityStable, Severity: SeverityError, Summary: "same component has both GOWDK and generated Go implementations"}, diff --git a/internal/gwdkast/ast.go b/internal/gwdkast/ast.go index 4a802b21..3ad83e5f 100644 --- a/internal/gwdkast/ast.go +++ b/internal/gwdkast/ast.go @@ -161,7 +161,12 @@ type Store struct { Name string Type GoTypeRef Init GoFuncRef - Span source.SourceSpan + // Persist is the optional `persist ""` modifier. It is empty when the + // store is not persisted, otherwise the raw (unquoted) scope literal. The + // scope value is validated later so an invalid literal still parses into a + // store and yields a precise diagnostic rather than a generic parse error. + Persist string + Span source.SourceSpan } // StateContract describes a component state type and initializer. diff --git a/internal/gwdkir/ir.go b/internal/gwdkir/ir.go index ef37113a..c1f3ef8c 100644 --- a/internal/gwdkir/ir.go +++ b/internal/gwdkir/ir.go @@ -81,7 +81,10 @@ type Store struct { Name string Type GoRef Init GoRef - Span source.SourceSpan + // Persist is the optional `persist ""` modifier scope literal + // ("" when not persisted). Validated in the compiler, not the parser. + Persist string + Span source.SourceSpan } // Page is the normalized IR for one page source. diff --git a/internal/lang/testdata/conformance/accept/store_persist.gwdk b/internal/lang/testdata/conformance/accept/store_persist.gwdk new file mode 100644 index 00000000..1df9fa1f --- /dev/null +++ b/internal/lang/testdata/conformance/accept/store_persist.gwdk @@ -0,0 +1,12 @@ +package pages + +import ui "github.com/cssbruno/gowdk/testfixture/islands" + +route "/shop" +guard public + +store cart ui.CounterState = ui.NewCounterState() persist "local" + +view { +
Shop
+} diff --git a/internal/lang/testdata/conformance/reject/store_persist_bad_scope.gwdk b/internal/lang/testdata/conformance/reject/store_persist_bad_scope.gwdk new file mode 100644 index 00000000..730acb28 --- /dev/null +++ b/internal/lang/testdata/conformance/reject/store_persist_bad_scope.gwdk @@ -0,0 +1,13 @@ +// expect: page_store_persist_scope_invalid +package pages + +import ui "github.com/cssbruno/gowdk/testfixture/islands" + +route "/shop" +guard public + +store cart ui.CounterState = ui.NewCounterState() persist "disk" + +view { +
Shop
+} diff --git a/internal/parser/page_lower.go b/internal/parser/page_lower.go index 68e45dff..dace7bfc 100644 --- a/internal/parser/page_lower.go +++ b/internal/parser/page_lower.go @@ -187,10 +187,11 @@ func lowerSyntaxStores(in []gwdkast.Store) []gwdkir.Store { out := make([]gwdkir.Store, 0, len(in)) for _, item := range in { out = append(out, gwdkir.Store{ - Name: item.Name, - Type: gwdkir.GoRef{Alias: item.Type.Alias, Name: item.Type.Name, Span: item.Type.Span}, - Init: gwdkir.GoRef{Alias: item.Init.Alias, Name: item.Init.Name, Span: item.Init.Span}, - Span: item.Span, + Name: item.Name, + Type: gwdkir.GoRef{Alias: item.Type.Alias, Name: item.Type.Name, Span: item.Type.Span}, + Init: gwdkir.GoRef{Alias: item.Init.Alias, Name: item.Init.Name, Span: item.Init.Span}, + Persist: item.Persist, + Span: item.Span, }) } return out diff --git a/internal/parser/page_test.go b/internal/parser/page_test.go index f3741064..1d2b353f 100644 --- a/internal/parser/page_test.go +++ b/internal/parser/page_test.go @@ -175,6 +175,45 @@ view { } } +func TestParsePageReadsStorePersistModifier(t *testing.T) { + for _, tc := range []struct { + name string + line string + scope string + }{ + {name: "local", line: `store cart ui.CounterState = ui.NewCounterState() persist "local"`, scope: "local"}, + {name: "session", line: `store cart ui.CounterState = ui.NewCounterState() persist "session"`, scope: "session"}, + {name: "no-persist", line: `store cart ui.CounterState = ui.NewCounterState()`, scope: ""}, + // An unknown scope still parses into a store so validation can emit a + // precise diagnostic rather than a generic parse error. + {name: "invalid-scope", line: `store cart ui.CounterState = ui.NewCounterState() persist "disk"`, scope: "disk"}, + } { + t.Run(tc.name, func(t *testing.T) { + page, err := ParsePage([]byte("\npage cart\nroute \"/cart\"\n\nimport ui \"github.com/cssbruno/gowdk/testfixture/islands\"\n\n" + tc.line + "\n\nview {\n
Cart
\n}\n")) + if err != nil { + t.Fatal(err) + } + if len(page.Stores) != 1 { + t.Fatalf("expected one store, got %#v", page.Stores) + } + if got := page.Stores[0].Persist; got != tc.scope { + t.Fatalf("store persist = %q, want %q", got, tc.scope) + } + if page.Stores[0].Name != "cart" || page.Stores[0].Init.Name != "NewCounterState" { + t.Fatalf("persist modifier corrupted the store: %#v", page.Stores[0]) + } + }) + } +} + +func TestParsePageStorePersistRequiresStringScope(t *testing.T) { + // `persist` without a string scope is not a valid store line. + page, err := ParsePage([]byte("\npage cart\nroute \"/cart\"\n\nimport ui \"github.com/cssbruno/gowdk/testfixture/islands\"\n\nstore cart ui.CounterState = ui.NewCounterState() persist\n\nview {\n
Cart
\n}\n")) + if err == nil && len(page.Stores) == 1 { + t.Fatalf("expected the bare persist keyword to be rejected, got store %#v", page.Stores[0]) + } +} + func TestParsePageReadsStyleBlockOutsideView(t *testing.T) { page, err := ParsePage([]byte(` page styled diff --git a/internal/parser/patterns.go b/internal/parser/patterns.go index 5528624e..24ce7d13 100644 --- a/internal/parser/patterns.go +++ b/internal/parser/patterns.go @@ -347,7 +347,9 @@ func parseComponentTypeLine(line string) []string { func parseStoreLine(line string) []string { tokens := syntaxTokens(line) - if len(tokens) != 6 || tokens[0].Kind != syntax.TokenIdentifier || tokens[0].Lexeme != "store" { + // 6 tokens: `store = ()`. + // 8 tokens: the same followed by `persist ""`. + if (len(tokens) != 6 && len(tokens) != 8) || tokens[0].Kind != syntax.TokenIdentifier || tokens[0].Lexeme != "store" { return nil } if tokens[1].Kind != syntax.TokenIdentifier || !isStrictIdent(tokens[1].Lexeme) || tokens[3].Kind != syntax.TokenAssign { @@ -357,11 +359,18 @@ func parseStoreLine(line string) []string { if !ok { return nil } - initAlias, initName, ok := parseQualifiedCall(tokens[4:]) + initAlias, initName, ok := parseQualifiedCall(tokens[4:6]) if !ok { return nil } - return []string{line, tokens[1].Lexeme, typeAlias, typeName, initAlias, initName} + persistScope := "" + if len(tokens) == 8 { + if tokens[6].Kind != syntax.TokenIdentifier || tokens[6].Lexeme != "persist" || tokens[7].Kind != syntax.TokenString { + return nil + } + persistScope = syntax.Unquote(tokens[7].Lexeme) + } + return []string{line, tokens[1].Lexeme, typeAlias, typeName, initAlias, initName, persistScope} } func parseActionInputLine(line string) []string { diff --git a/internal/parser/syntax.go b/internal/parser/syntax.go index 4ec6551e..b47ee14a 100644 --- a/internal/parser/syntax.go +++ b/internal/parser/syntax.go @@ -252,10 +252,11 @@ func ParseSyntax(src []byte) (SyntaxFile, error) { if match := storePattern.FindStringSubmatch(line); match != nil { span := sourceLineSpan(lineNumber, rawLine) file.Stores = append(file.Stores, gwdkast.Store{ - Name: match[1], - Type: GoTypeRef{Alias: match[2], Name: match[3], Span: span}, - Init: GoFuncRef{Alias: match[4], Name: match[5], Span: span}, - Span: span, + Name: match[1], + Type: GoTypeRef{Alias: match[2], Name: match[3], Span: span}, + Init: GoFuncRef{Alias: match[4], Name: match[5], Span: span}, + Persist: match[6], + Span: span, }) continue } diff --git a/internal/syntax/declparse.go b/internal/syntax/declparse.go index 046896ed..d465e9c9 100644 --- a/internal/syntax/declparse.go +++ b/internal/syntax/declparse.go @@ -187,11 +187,25 @@ func parseStoreTokens(src string, line []Token, end Token) (gwdkast.Store, bool) if !ok { return gwdkast.Store{}, false } - initRef, ok := goFuncRef(sourceBetween(src, tokenEnd(line[assign]), end.Offset), span) + // An optional trailing `persist ""` clause bounds the initializer: the + // init expression runs from `=` up to the `persist` keyword (or end of line). + persistScope := "" + initStop := end.Offset + for index := assign + 1; index < len(line); index++ { + if line[index].Kind == TokenIdentifier && line[index].Lexeme == "persist" { + if index+1 >= len(line) || line[index+1].Kind != TokenString { + return gwdkast.Store{}, false + } + persistScope = Unquote(line[index+1].Lexeme) + initStop = line[index].Offset + break + } + } + initRef, ok := goFuncRef(sourceBetween(src, tokenEnd(line[assign]), initStop), span) if !ok { return gwdkast.Store{}, false } - return gwdkast.Store{Name: line[1].Lexeme, Type: typeRef, Init: initRef, Span: span}, true + return gwdkast.Store{Name: line[1].Lexeme, Type: typeRef, Init: initRef, Persist: persistScope, Span: span}, true } // parsePropsTokens recovers a `props ` contract: a type reference with diff --git a/testfixture/islands/islands.go b/testfixture/islands/islands.go index 09204952..6d1cf252 100644 --- a/testfixture/islands/islands.go +++ b/testfixture/islands/islands.go @@ -44,10 +44,21 @@ type FilterState struct { Items []Item } +// SessionState carries a secret-resembling field. It exists to exercise the +// persisted-store secret-field warning and should not be persisted in practice. +type SessionState struct { + Token string + Open bool +} + func NewCounterState() CounterState { return CounterState{Count: 1, Open: false} } +func NewSessionState() SessionState { + return SessionState{} +} + func NewOtherState() OtherState { return OtherState{Name: "other"} } From 32c10568f8250f024a6df67bb2475aaa602f888b Mon Sep 17 00:00:00 2001 From: Bruno Carvalho Date: Sat, 13 Jun 2026 12:16:02 -0300 Subject: [PATCH 2/5] fix(stores): close persistence fallbacks flagged in PR review 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. --- CHANGELOG.md | 12 +++-- docs/language/components.md | 14 ++++-- docs/language/syntax.md | 4 +- internal/buildgen/runtime_assets.go | 19 ++++++- .../buildgen/store_persist_runtime_test.go | 31 ++++++++++++ internal/compiler/validate_page.go | 37 ++++++++++++-- .../compiler/validate_page_persist_test.go | 47 ++++++++++++++++++ internal/compiler/validate_stores.go | 49 +++++++++++++------ internal/diagnostics/explain.go | 15 ++++++ internal/diagnostics/registry.go | 1 + testfixture/islands/islands.go | 19 +++++++ 11 files changed, 220 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ece59d..d6907de9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,14 +12,20 @@ packages, and tooling contracts may change before a stable release. (`store cart ui.CartState = ui.NewCartState() persist "local"`). The generated store runtime hydrates from localStorage/sessionStorage on load, re-hydrates on SPA navigation (stores first declared on a later route are picked up on content - swap), writes the store's declared fields on change, mirrors cross-tab writes + swap, and a store first declared without persistence adopts a later route's + persist config and restores its saved value, so persistence never depends on + navigation order), writes the store's declared fields on change, mirrors cross-tab writes through the `storage` event, exposes `window.__gowdkStores.clear(name)` to drop a persisted store, and discards persisted state whose embedded schema hash no longer matches the store's shape (so a struct change never restores stale data). Only the store's own 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). Persistence is a JS-island/store + `page_store_persist_secret_field` (warning, raised for nested secret-resembling + fields such as `Profile.Token`, not only top-level fields), + `page_store_persist_key_conflict` (warning), and + `page_store_persist_scope_conflict` (warning, when the same store name is + persisted under different `local`/`session` scopes across pages and would + otherwise let navigation order decide the backend). Persistence is a JS-island/store runtime feature; WASM islands do not yet participate in page stores. - M4 Go interop is complete for the current 0.x surface: a user can see why a Go function or type did or did not bind. `gowdk inspect go-bindings` emits a diff --git a/docs/language/components.md b/docs/language/components.md index 8c1d3f8b..7f2439e8 100644 --- a/docs/language/components.md +++ b/docs/language/components.md @@ -293,9 +293,11 @@ component state, props, or computed values. The compiler embeds a hash of the store's struct shape; when the shape changes, stale persisted state is discarded rather than restored, so a struct change never crashes on old data. Because browser storage is readable by any script on the origin, persisting a field whose -name resembles a secret (`token`, `password`, `secret`, `auth`, …) is a warning: -keep credentials and trusted authorization state server-side. An unknown scope is -rejected — see `gowdk explain page_store_persist_scope_invalid`. +name resembles a secret (`token`, `password`, `secret`, `auth`, …) is a warning — +including a nested field such as `Profile.Token`, because persistence writes the +whole value of each top-level field: keep credentials and trusted authorization +state server-side. An unknown scope is rejected — see +`gowdk explain page_store_persist_scope_invalid`. Persisted stores also sync across tabs: when one tab writes, other tabs on the origin mirror the value through the browser `storage` event. To drop a persisted @@ -304,7 +306,11 @@ store (for example after checkout or logout), call the store to its build-time init value. If two pages persist a store with the same name but different shapes, they share one storage key and discard each other's data on navigation; the compiler warns with -`page_store_persist_key_conflict`. +`page_store_persist_key_conflict`. If they share the same shape but declare +different `local`/`session` scopes, the runtime keeps whichever scope initialized +first and the compiler warns with `page_store_persist_scope_conflict`. A store +first reached on a route that does not persist it still adopts persistence when a +later route declares it, restoring the saved value regardless of navigation order. Persistence survives SPA navigation: when the client runtime swaps page content it re-scans store seeds, so a store first declared on a later client-side route diff --git a/docs/language/syntax.md b/docs/language/syntax.md index 684bacb4..1989b812 100644 --- a/docs/language/syntax.md +++ b/docs/language/syntax.md @@ -499,7 +499,9 @@ The scope must be `"local"` (localStorage) or `"session"` (sessionStorage); any other value is rejected with `page_store_persist_scope_invalid`. Persisted store state is keyed by store name, restores over the build-time init value on load, is discarded when the store's struct shape changes, and warns when a persisted -field name resembles a secret. +field name resembles a secret (nested fields included). Declaring the same store +name with a different `local`/`session` scope across pages warns with +`page_store_persist_scope_conflict`. Client blocks can declare limited DOM refs for safe methods: diff --git a/internal/buildgen/runtime_assets.go b/internal/buildgen/runtime_assets.go index fff663c9..2adb4c91 100644 --- a/internal/buildgen/runtime_assets.go +++ b/internal/buildgen/runtime_assets.go @@ -138,7 +138,24 @@ func storeRuntimeSource() string { }; registry.init = (name, state, persist) => { - if (!name || registry.stores[name]) return; + if (!name) return; + if (registry.stores[name]) { + // The store already exists (for example SPA navigation reached a later + // route that declares the same store). If this declaration adds + // persistence and we are not persisting yet, adopt it and restore the + // 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]) { + registry.persist[name] = persist; + const restored = readPersisted(persist, registry.fields[name]); + if (restored) { + registry.stores[name] = Object.assign({}, registry.stores[name] || {}, restored); + notify(name); + } + } + return; + } const seed = Object.assign(Object.create(null), state || {}); registry.fields[name] = Object.keys(seed); registry.seeds[name] = Object.assign(Object.create(null), seed); diff --git a/internal/buildgen/store_persist_runtime_test.go b/internal/buildgen/store_persist_runtime_test.go index de542d19..de8bf699 100644 --- a/internal/buildgen/store_persist_runtime_test.go +++ b/internal/buildgen/store_persist_runtime_test.go @@ -163,6 +163,37 @@ r.hydrate(); assert.equal(r.get("prefs").Theme, "dark", "hydrate() picks up a store first seen on a later route"); assert.equal(r.get("cart").Count, 0, "re-hydrate leaves an existing store untouched"); +// 9. Adopting persistence across SPA navigation: a store first declared WITHOUT +// persistence must pick up a later route's persist config (and restore the saved +// value) instead of silently staying unpersisted, so persistence does not depend +// on which route loaded first. +localStorage.setItem("gowdk:store:wishlist", JSON.stringify({ v: "w1", s: { Items: 2 } })); +// First navigation declares wishlist without persistence (no data-gowdk-persist). +global.document.querySelectorAll = () => [{ + getAttribute(name) { return name === "data-gowdk-store" ? "wishlist" : null; }, + get textContent() { return '{"Items":0}'; } +}]; +r.hydrate(); +assert.equal(r.get("wishlist").Items, 0, "unpersisted first declaration uses the seed, not storage"); +// Second navigation declares the same store as persist "local". +let adopted = null; +r.subscribe("wishlist", (next) => { adopted = next; }); +global.document.querySelectorAll = () => [{ + getAttribute(name) { + if (name === "data-gowdk-store") return "wishlist"; + if (name === "data-gowdk-persist") return "local"; + if (name === "data-gowdk-persist-key") return "gowdk:store:wishlist"; + if (name === "data-gowdk-persist-version") return "w1"; + return null; + }, + get textContent() { return '{"Items":0}'; } +}]; +r.hydrate(); +assert.equal(r.get("wishlist").Items, 2, "re-hydrate adopts persistence and restores the saved value"); +assert.ok(adopted && adopted.Items === 2, "adopting persistence notifies subscribers"); +r.set("wishlist", { Items: 7 }); +assert.equal(JSON.parse(localStorage.getItem("gowdk:store:wishlist")).s.Items, 7, "after adoption, set() writes through to storage"); + console.log("OK"); ` } diff --git a/internal/compiler/validate_page.go b/internal/compiler/validate_page.go index 151393be..3146402c 100644 --- a/internal/compiler/validate_page.go +++ b/internal/compiler/validate_page.go @@ -3,6 +3,7 @@ package compiler import ( "fmt" "os" + "sort" "strings" "github.com/cssbruno/gowdk" @@ -419,22 +420,48 @@ func validateStorePersist(page gwdkir.Page, store gwdkir.Store, resolved gotypes }} } var diagnostics []ValidationError - for _, field := range resolved.Fields { - if !looksLikeSecretFieldName(field.Name) { - continue - } + // Persistence writes the whole value of each top-level field, so a nested + // field such as Profile.Token reaches browser storage too. Scan every field + // path the resolver recorded (top-level and nested), not just the top level. + for _, path := range secretResemblingFieldPaths(resolved) { diagnostics = append(diagnostics, ValidationError{ Code: "page_store_persist_secret_field", PageID: page.ID, Source: page.Source, Span: firstSpan(store.Span, page.Spans.Page), Severity: SeverityWarning, - Message: fmt.Sprintf("page %s store %q persists field %q, which resembles a secret; %s browser storage is readable by any script on this origin", page.ID, store.Name, field.Name, store.Persist), + Message: fmt.Sprintf("page %s store %q persists field %q, which resembles a secret; %s browser storage is readable by any script on this origin", page.ID, store.Name, path, store.Persist), }) } return diagnostics } +// secretResemblingFieldPaths returns the resolved struct's field paths (top-level +// and nested) whose leaf name resembles a secret, deduplicated and sorted for a +// stable diagnostic order. Slice/array markers ("[]") are stripped so a path +// reads like Tags.Token rather than Tags[].Token. +func secretResemblingFieldPaths(resolved gotypes.Struct) []string { + seen := map[string]bool{} + var paths []string + for raw := range resolved.FieldTypes { + path := strings.ReplaceAll(raw, "[]", "") + leaf := path + if index := strings.LastIndex(path, "."); index >= 0 { + leaf = path[index+1:] + } + if leaf == "" || !looksLikeSecretFieldName(leaf) { + continue + } + if seen[path] { + continue + } + seen[path] = true + paths = append(paths, path) + } + sort.Strings(paths) + return paths +} + // looksLikeSecretFieldName flags field names that commonly hold credentials or // trusted authorization state, which the store contract already forbids from // browser-visible state and which persistence would write to disk. diff --git a/internal/compiler/validate_page_persist_test.go b/internal/compiler/validate_page_persist_test.go index 8ac089c4..29eee9f1 100644 --- a/internal/compiler/validate_page_persist_test.go +++ b/internal/compiler/validate_page_persist_test.go @@ -1,6 +1,7 @@ package compiler import ( + "strings" "testing" "github.com/cssbruno/gowdk" @@ -88,6 +89,52 @@ func TestValidatePageWarnsOnPersistedSecretField(t *testing.T) { } } +func TestValidatePageWarnsOnNestedPersistedSecretField(t *testing.T) { + // ProfileState has no top-level secret-like field, but nests Account.Token. + // Persistence writes the whole Account value, so the nested secret must be + // flagged even though the top-level scan alone would miss it. + page := persistedStorePage("profile", "ProfileState", "NewProfileState", "local") + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + diagnostic := firstDiagnostic(diagnostics, "page_store_persist_secret_field") + if diagnostic == nil { + t.Fatalf("missing nested page_store_persist_secret_field diagnostic: %#v", diagnostics) + } + if !strings.Contains(diagnostic.Message, "Account.Token") { + t.Fatalf("expected the nested field path Account.Token in the message, got %q", diagnostic.Message) + } + if diagnostic.Severity != SeverityWarning { + t.Fatalf("nested secret field should be a warning, got severity %v", diagnostic.Severity) + } + if ValidationErrors(diagnostics).HasErrors() { + t.Fatalf("nested secret field warning should not fail the build: %#v", diagnostics) + } +} + +func TestValidateWarnsOnPersistedStoreScopeConflict(t *testing.T) { + // Same store name and shape but different persist scopes share one storage + // key; the effective scope then depends on navigation order. + app := appFixture{Pages: []gwdkir.Page{ + persistedStorePageNamed("shop", "/shop", "cart", "CounterState", "NewCounterState", "local"), + persistedStorePageNamed("checkout", "/checkout", "cart", "CounterState", "NewCounterState", "session"), + }} + diagnostics := ValidateProgramReport(gowdk.Config{}, app.program(gowdk.Config{})) + d := firstDiagnostic(diagnostics, "page_store_persist_scope_conflict") + if d == nil { + t.Fatalf("missing page_store_persist_scope_conflict: %#v", diagnostics) + } + if d.Severity != SeverityWarning { + t.Fatalf("scope conflict should be a warning, got %v", d.Severity) + } + // A mismatched-shape conflict is reported separately; same shape must not + // also raise a key conflict. + if other := firstDiagnostic(diagnostics, "page_store_persist_key_conflict"); other != nil { + t.Fatalf("same-shape scope conflict must not also raise a key conflict: %#v", other) + } + if ValidationErrors(diagnostics).HasErrors() { + t.Fatalf("scope conflict alone must not fail the build: %#v", diagnostics) + } +} + func TestValidatePageDoesNotPersistCheckUnpersistedStore(t *testing.T) { // A non-persisted store with a secret-looking field must stay silent. page := persistedStorePage("session", "SessionState", "NewSessionState", "") diff --git a/internal/compiler/validate_stores.go b/internal/compiler/validate_stores.go index 6b0172eb..c30b711d 100644 --- a/internal/compiler/validate_stores.go +++ b/internal/compiler/validate_stores.go @@ -36,21 +36,42 @@ func validatePersistedStoreConflicts(pages []gwdkir.Page) []ValidationError { seen[store.Name] = seenStore{page: page, store: store, signature: signature} continue } - if prior.signature == signature { - continue // same shape sharing one key across routes is intended + if prior.signature != signature { + diagnostics = append(diagnostics, ValidationError{ + Code: "page_store_persist_key_conflict", + PageID: page.ID, + Source: page.Source, + Span: firstSpan(store.Span, page.Spans.Page), + Severity: SeverityWarning, + Related: relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted here on page %s", prior.store.Name, prior.page.ID)), + Message: fmt.Sprintf( + "persisted store %q has different shapes on pages %s and %s but shares browser storage key %q; navigating between them discards each other's saved data. Rename one store or give them matching shapes", + store.Name, prior.page.ID, page.ID, "gowdk:store:"+store.Name, + ), + }) + continue } - diagnostics = append(diagnostics, ValidationError{ - Code: "page_store_persist_key_conflict", - PageID: page.ID, - Source: page.Source, - Span: firstSpan(store.Span, page.Spans.Page), - Severity: SeverityWarning, - Related: relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted here on page %s", prior.store.Name, prior.page.ID)), - Message: fmt.Sprintf( - "persisted store %q has different shapes on pages %s and %s but shares browser storage key %q; navigating between them discards each other's saved data. Rename one store or give them matching shapes", - store.Name, prior.page.ID, page.ID, "gowdk:store:"+store.Name, - ), - }) + if prior.store.Persist != store.Persist { + // Same name and shape but different persist scopes (local vs + // session) still share one browser storage key. The runtime + // registry keeps the scope of whichever route initialized first, + // so the effective backend depends on navigation history. Flag + // it instead of silently letting nav order decide. + diagnostics = append(diagnostics, ValidationError{ + Code: "page_store_persist_scope_conflict", + PageID: page.ID, + Source: page.Source, + Span: firstSpan(store.Span, page.Spans.Page), + Severity: SeverityWarning, + Related: relatedSpan(prior.page.Source, prior.store.Span, fmt.Sprintf("store %q first persisted with scope %q here on page %s", prior.store.Name, prior.store.Persist, prior.page.ID)), + Message: fmt.Sprintf( + "persisted store %q uses scope %q on page %s but scope %q on page %s while sharing browser storage key %q; only one scope wins and which one depends on navigation order. Use the same persist scope on both pages or rename one store", + store.Name, prior.store.Persist, prior.page.ID, store.Persist, page.ID, "gowdk:store:"+store.Name, + ), + }) + continue + } + // Same name, shape, and scope is intentional cross-route sharing. } } return diagnostics diff --git a/internal/diagnostics/explain.go b/internal/diagnostics/explain.go index e1956328..ffcd1865 100644 --- a/internal/diagnostics/explain.go +++ b/internal/diagnostics/explain.go @@ -41,6 +41,21 @@ store cart admin.AuditState = admin.NewAuditState() persist "local"`, store cart ui.CartState = ui.NewCartState() persist "local" // pages/admin.page.gwdk store audit admin.AuditState = admin.NewAuditState() persist "local"`, + }, + "page_store_persist_scope_conflict": { + Details: "Two pages declare a persisted store with the same name and the same struct shape but different persist scopes (one local, one session). Persistence is keyed by store name (gowdk:store:), so they share one storage slot, but local uses window.localStorage and session uses window.sessionStorage. The runtime keeps whichever scope initialized first, so the effective backend — and whether the value survives a browser restart — depends on which route the user visited first. Use the same scope on both pages, or rename one store.", + NextSteps: []string{ + "Declare the same persist scope (both \"local\" or both \"session\") wherever the store is persisted.", + "Rename one of the stores so each persisted store owns its own scope and storage key.", + }, + Invalid: `// pages/shop.page.gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +// pages/checkout.page.gwdk (same shape, different scope) +store cart ui.CartState = ui.NewCartState() persist "session"`, + Fixed: `// pages/shop.page.gwdk +store cart ui.CartState = ui.NewCartState() persist "local" +// pages/checkout.page.gwdk +store cart ui.CartState = ui.NewCartState() persist "local"`, }, "page_store_persist_scope_invalid": { Details: "A page store may opt into browser persistence with persist \"local\" or persist \"session\". local uses window.localStorage (survives a browser restart); session uses window.sessionStorage (survives reload and SPA navigation, cleared when the tab closes). No other scope is supported.", diff --git a/internal/diagnostics/registry.go b/internal/diagnostics/registry.go index b3d35183..0a725e0f 100644 --- a/internal/diagnostics/registry.go +++ b/internal/diagnostics/registry.go @@ -131,6 +131,7 @@ var Registry = []Code{ {Code: "package_must_be_first", Area: "packages", Stability: StabilityStable, Severity: SeverityError, Summary: "package declaration is not the first non-comment declaration"}, {Code: "page_store_error", Area: "stores", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page store type or initializer is invalid"}, {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"}, {Code: "parse_error", Area: "parser", Stability: StabilityStable, Severity: SeverityError, Summary: "parser rejected source without a more specific code"}, diff --git a/testfixture/islands/islands.go b/testfixture/islands/islands.go index 6d1cf252..ba4e1f73 100644 --- a/testfixture/islands/islands.go +++ b/testfixture/islands/islands.go @@ -51,6 +51,21 @@ type SessionState struct { Open bool } +// Credentials nests a secret-resembling field. It exercises the nested +// persisted-store secret-field warning and should not be persisted in practice. +type Credentials struct { + Token string + Label string +} + +// ProfileState carries a nested Credentials value, so persisting it would write +// the nested Token to browser storage even though no top-level field looks like +// a secret. +type ProfileState struct { + Name string + Account Credentials +} + func NewCounterState() CounterState { return CounterState{Count: 1, Open: false} } @@ -59,6 +74,10 @@ func NewSessionState() SessionState { return SessionState{} } +func NewProfileState() ProfileState { + return ProfileState{} +} + func NewOtherState() OtherState { return OtherState{Name: "other"} } From 225e8d3d1dcbf919870a40ba883bb34da43523c0 Mon Sep 17 00:00:00 2001 From: cssbruno Date: Sat, 13 Jun 2026 15:06:07 -0300 Subject: [PATCH 3/5] fix(stores): address persist review follow-ups - 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 --- docs/reference/diagnostic-codes.md | 2 + examples/store-persist/README.md | 2 +- examples/store-persist/ui/cart.go | 6 +- internal/buildgen/runtime_assets.go | 61 ++++++++++++++++--- .../buildgen/store_persist_runtime_test.go | 48 +++++++++++++++ internal/compiler/validate_stores.go | 14 ++++- 6 files changed, 118 insertions(+), 15 deletions(-) diff --git a/docs/reference/diagnostic-codes.md b/docs/reference/diagnostic-codes.md index db9aea52..a0423679 100644 --- a/docs/reference/diagnostic-codes.md +++ b/docs/reference/diagnostic-codes.md @@ -132,6 +132,8 @@ Parser diagnostics emit stable codes for common unsupported syntax and keep `redundant_component_implementation`, `component_contract_error`, `component_field_error`, `component_client_error`, `duplicate_component_emit`, `duplicate_page_store`, `page_store_error`, + `page_store_persist_key_conflict`, `page_store_persist_scope_conflict`, + `page_store_persist_scope_invalid`, `page_store_persist_secret_field`, `unknown_component_store`, `view_parse_error`. - Accessibility: `missing_img_alt`. - Go blocks and generated app wiring: `invalid_go_block`, diff --git a/examples/store-persist/README.md b/examples/store-persist/README.md index 81162eeb..0a182aa3 100644 --- a/examples/store-persist/README.md +++ b/examples/store-persist/README.md @@ -28,7 +28,7 @@ The generated `shop/index.html` carries the persist config on the store seed: + data-gowdk-persist-version="…">{"Count":0} ``` and `assets/gowdk/islands/stores.js` hydrates the store from `localStorage` on diff --git a/examples/store-persist/ui/cart.go b/examples/store-persist/ui/cart.go index 95d7c131..55a9f678 100644 --- a/examples/store-persist/ui/cart.go +++ b/examples/store-persist/ui/cart.go @@ -3,9 +3,11 @@ package ui // CartState is the cart store's Go type. When the page store opts into -// persist "local", only these declared fields are written to browser storage. +// persist "local", only these declared fields are written to browser storage, +// keyed by their exported Go field name (so the seed and persisted blob use +// "Count", not a json tag). type CartState struct { - Count int `json:"count"` + Count int } // NewCartState is the build-time initializer for the cart store. diff --git a/internal/buildgen/runtime_assets.go b/internal/buildgen/runtime_assets.go index 2adb4c91..f9a14ba2 100644 --- a/internal/buildgen/runtime_assets.go +++ b/internal/buildgen/runtime_assets.go @@ -89,6 +89,16 @@ func storeRuntimeSource() string { return projected; }; + const sameFieldSet = (left, right) => { + if (left.length !== right.length) return false; + const sortedLeft = left.slice().sort(); + const sortedRight = right.slice().sort(); + for (let index = 0; index < sortedLeft.length; index++) { + if (sortedLeft[index] !== sortedRight[index]) return false; + } + return true; + }; + const decodePersisted = (config, fields, raw) => { if (!raw) return null; let blob = null; @@ -139,14 +149,42 @@ func storeRuntimeSource() string { registry.init = (name, state, persist) => { if (!name) return; + const seed = Object.assign(Object.create(null), state || {}); + const fields = Object.keys(seed); + const hasPersist = !!(persist && persist.scope && persist.key && persist.version); + if (registry.stores[name]) { // The store already exists (for example SPA navigation reached a later - // route that declares the same store). If this declaration adds - // persistence and we are not persisting yet, adopt it and restore the - // 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 + // route that declares the same store). + const prior = registry.persist[name]; + // If the later route changes the field set, or the persisted version + // (shape hash) of an already-persisted store, re-seed so the current + // route's islands read the fields and version they declared and stale + // storage written under the old shape is discarded. Divergent shapes that + // share a storage key are reported at build time by + // page_store_persist_key_conflict. + const shapeChanged = + !sameFieldSet(registry.fields[name] || [], fields) || + (hasPersist && prior && prior.version !== persist.version); + if (shapeChanged) { + registry.fields[name] = fields; + registry.seeds[name] = Object.assign(Object.create(null), seed); + delete registry.persist[name]; + if (hasPersist) { + registry.persist[name] = persist; + const restored = readPersisted(persist, fields); + if (restored) Object.assign(seed, restored); + } + registry.stores[name] = seed; + notify(name); + return; + } + // Same shape: if this declaration adds persistence and we are not + // persisting yet, adopt it and restore the 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]) { + if (hasPersist && !prior) { registry.persist[name] = persist; const restored = readPersisted(persist, registry.fields[name]); if (restored) { @@ -156,12 +194,12 @@ func storeRuntimeSource() string { } return; } - const seed = Object.assign(Object.create(null), state || {}); - registry.fields[name] = Object.keys(seed); + + registry.fields[name] = fields; registry.seeds[name] = Object.assign(Object.create(null), seed); - if (persist && persist.scope && persist.key && persist.version) { + if (hasPersist) { registry.persist[name] = persist; - const restored = readPersisted(persist, registry.fields[name]); + const restored = readPersisted(persist, fields); if (restored) Object.assign(seed, restored); } registry.stores[name] = seed; @@ -212,6 +250,11 @@ func storeRuntimeSource() string { Object.keys(registry.persist).forEach((name) => { const config = registry.persist[name]; if (!config || config.key !== event.key) return; + // Local and session stores can share the gowdk:store: key. Ignore + // events from the other storage area so a localStorage write never + // overwrites a sessionStorage-backed store (or vice versa). Older + // browsers omit storageArea, where the key match alone is used. + if (event.storageArea && event.storageArea !== storageFor(config.scope)) return; if (event.newValue == null) { if (registry.seeds[name]) registry.stores[name] = Object.assign({}, registry.seeds[name]); } else { diff --git a/internal/buildgen/store_persist_runtime_test.go b/internal/buildgen/store_persist_runtime_test.go index de8bf699..2d2d974c 100644 --- a/internal/buildgen/store_persist_runtime_test.go +++ b/internal/buildgen/store_persist_runtime_test.go @@ -194,6 +194,54 @@ assert.ok(adopted && adopted.Items === 2, "adopting persistence notifies subscri r.set("wishlist", { Items: 7 }); assert.equal(JSON.parse(localStorage.getItem("gowdk:store:wishlist")).s.Items, 7, "after adoption, set() writes through to storage"); +// 10. A later route re-declares an existing persisted store with a different +// shape and version. The runtime must re-seed to the new field set and discard +// storage written under the old version, so the current route's islands read the +// fields they declared instead of the prior route's (build-time +// page_store_persist_key_conflict warns on the divergence). +localStorage.setItem("gowdk:store:profile", JSON.stringify({ v: "a1", s: { Name: "ada", Theme: "dark" } })); +global.document.querySelectorAll = () => [{ + getAttribute(name) { + if (name === "data-gowdk-store") return "profile"; + if (name === "data-gowdk-persist") return "local"; + if (name === "data-gowdk-persist-key") return "gowdk:store:profile"; + if (name === "data-gowdk-persist-version") return "a1"; + return null; + }, + get textContent() { return '{"Name":"","Theme":"light"}'; } +}]; +r.hydrate(); +assert.equal(r.get("profile").Theme, "dark", "precondition: restored saved value under version a1"); +let reseeded = null; +r.subscribe("profile", (next) => { reseeded = next; }); +global.document.querySelectorAll = () => [{ + getAttribute(name) { + if (name === "data-gowdk-store") return "profile"; + if (name === "data-gowdk-persist") return "local"; + if (name === "data-gowdk-persist-key") return "gowdk:store:profile"; + if (name === "data-gowdk-persist-version") return "a2"; + return null; + }, + get textContent() { return '{"Name":"","Density":"cozy"}'; } +}]; +r.hydrate(); +assert.equal(r.get("profile").Density, "cozy", "shape/version change re-seeds to the new field set"); +assert.ok(!("Theme" in r.get("profile")), "the dropped field is gone after a re-seed"); +assert.equal(JSON.parse(localStorage.getItem("gowdk:store:profile") || "{}").v, "a1", "stale storage stays under the old version until the next write"); +assert.ok(reseeded && reseeded.Density === "cozy", "a shape/version change notifies subscribers"); +r.set("profile", { Name: "ada", Density: "compact" }); +assert.equal(JSON.parse(localStorage.getItem("gowdk:store:profile")).v, "a2", "after a re-seed, writes persist under the new version"); + +// 11. A storage event from a different storage area must be ignored so local and +// session stores that share a key cannot cross-contaminate. +global.document.querySelectorAll = () => [makeNode()]; +r = boot(); +r.set("cart", { Count: 1, Open: false }); +storageListeners[0]({ key: "gowdk:store:cart", newValue: JSON.stringify({ v: "v1", s: { Count: 99, Open: true } }), storageArea: sessionStorage }); +assert.equal(r.get("cart").Count, 1, "a storage event from a different area (session) is ignored by a local store"); +storageListeners[0]({ key: "gowdk:store:cart", newValue: JSON.stringify({ v: "v1", s: { Count: 99, Open: true } }), storageArea: localStorage }); +assert.equal(r.get("cart").Count, 99, "a storage event from the matching area is applied"); + console.log("OK"); ` } diff --git a/internal/compiler/validate_stores.go b/internal/compiler/validate_stores.go index c30b711d..4c45aa0a 100644 --- a/internal/compiler/validate_stores.go +++ b/internal/compiler/validate_stores.go @@ -77,14 +77,22 @@ func validatePersistedStoreConflicts(pages []gwdkir.Page) []ValidationError { return diagnostics } +// persistedStoreSignature folds in the resolved struct's full field-type map +// (every top-level and nested path, keyed by name) so the conflict signature +// tracks the same Go-shape inputs as buildgen.storeSchemaHash, which seeds the +// runtime persist version. Comparing only top-level fields would miss a nested +// shape change that still rotates the version hash and makes the runtime discard +// the other page's saved value. The reflective seed encoder keys JSON by Go +// field name, so the on-wire keys storeSchemaHash also folds in equal the +// top-level field names already covered here. func persistedStoreSignature(page gwdkir.Page, store gwdkir.Store) string { resolved, err := gotypes.ResolveStruct(page.Imports, store.Type) if err != nil { return "" } - parts := make([]string, 0, len(resolved.Fields)) - for _, field := range resolved.Fields { - parts = append(parts, field.Name+":"+field.Type) + parts := make([]string, 0, len(resolved.FieldTypes)) + for name, fieldType := range resolved.FieldTypes { + parts = append(parts, name+":"+fieldType) } sort.Strings(parts) return strings.Join(parts, ",") From 7381f696a22ad52f42066a3f7ea6f7b35800972e Mon Sep 17 00:00:00 2001 From: cssbruno Date: Sat, 13 Jun 2026 15:45:44 -0300 Subject: [PATCH 4/5] fix(stores): close second-round persist review gaps - 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. --- CHANGELOG.md | 3 +- docs/language/components.md | 7 ++- internal/buildgen/islands_test.go | 2 +- internal/buildgen/render.go | 9 ++- internal/buildgen/runtime_assets.go | 57 ++++++++++--------- .../buildgen/store_persist_runtime_test.go | 45 +++++++++++++++ internal/clientrt/runtime.go | 30 +++++++++- internal/clientrt/runtime_test.go | 8 +++ internal/compiler/validate_page.go | 5 +- .../compiler/validate_page_persist_test.go | 34 ++++++++--- internal/gwdkast/ast.go | 7 ++- internal/gwdkir/ir.go | 5 +- internal/parser/page_lower.go | 11 ++-- internal/parser/page_test.go | 21 ++++--- internal/parser/patterns.go | 6 +- internal/parser/syntax.go | 11 ++-- internal/syntax/declparse.go | 4 +- 17 files changed, 201 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6907de9..80a23500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ packages, and tooling contracts may change before a stable release. swap, and a store first declared without persistence adopts a later route's persist config and restores its saved value, so persistence never depends on navigation order), writes the store's declared fields on change, mirrors cross-tab writes - through the `storage` event, exposes `window.__gowdkStores.clear(name)` to drop + for `persist "local"` stores through the `storage` event (`persist "session"` + stores are tab-local), exposes `window.__gowdkStores.clear(name)` to drop a persisted store, and discards persisted state whose embedded schema hash no longer matches the store's shape (so a struct change never restores stale data). Only the store's own fields persist — never component state, props, or computed diff --git a/docs/language/components.md b/docs/language/components.md index 7f2439e8..826f115c 100644 --- a/docs/language/components.md +++ b/docs/language/components.md @@ -299,8 +299,11 @@ whole value of each top-level field: keep credentials and trusted authorization state server-side. An unknown scope is rejected — see `gowdk explain page_store_persist_scope_invalid`. -Persisted stores also sync across tabs: when one tab writes, other tabs on the -origin mirror the value through the browser `storage` event. To drop a persisted +`persist "local"` stores also sync across tabs: when one tab writes, other tabs +on the origin mirror the value through the browser `storage` event. `persist +"session"` stores are deliberately tab-local — `sessionStorage` is partitioned +per top-level tab, so session-scoped stores do not (and cannot) sync across tabs. +To drop a persisted store (for example after checkout or logout), call `window.__gowdkStores.clear("")`, which removes the stored copy and resets the store to its build-time init value. If two pages persist a store with the diff --git a/internal/buildgen/islands_test.go b/internal/buildgen/islands_test.go index 4f6de8fe..9944ccc8 100644 --- a/internal/buildgen/islands_test.go +++ b/internal/buildgen/islands_test.go @@ -176,7 +176,7 @@ fn Add() { html := readFile(t, filepath.Join(outputDir, "counter", "index.html")) for _, expected := range []string{ ``, - ``, + ``, ``, `data-gowdk-client="{"handlers":{"Add":{"statements":["Count++"]}},"stores":["cart"]}"`, } { diff --git a/internal/buildgen/render.go b/internal/buildgen/render.go index 393613d9..db5105b4 100644 --- a/internal/buildgen/render.go +++ b/internal/buildgen/render.go @@ -444,7 +444,14 @@ func document(config gowdk.Config, page gwdkir.Page, body string, stylesheets [] if strings.TrimSpace(script.Type) != "" { tag += gowhtml.Attr("type", script.Type) } - tag += gowhtml.Attr("src", script.Src) + " defer>" + tag += gowhtml.Attr("src", script.Src) + // Mark the store runtime so the SPA navigation runtime can run (and + // hydrate) it before island bundles, which auto-mount on execution and read + // the store registry during mount. + if script.Src == storeRuntimeHref { + tag += " data-gowdk-store-runtime" + } + tag += " defer>" head = append(head, tag) } head = append(head, "") diff --git a/internal/buildgen/runtime_assets.go b/internal/buildgen/runtime_assets.go index f9a14ba2..0631dd7f 100644 --- a/internal/buildgen/runtime_assets.go +++ b/internal/buildgen/runtime_assets.go @@ -157,15 +157,20 @@ func storeRuntimeSource() string { // The store already exists (for example SPA navigation reached a later // route that declares the same store). const prior = registry.persist[name]; - // If the later route changes the field set, or the persisted version - // (shape hash) of an already-persisted store, re-seed so the current - // route's islands read the fields and version they declared and stale - // storage written under the old shape is discarded. Divergent shapes that - // share a storage key are reported at build time by - // page_store_persist_key_conflict. + // Re-seed when the field set changes, when an already-persisted store's + // version (shape hash) changes, OR when this declaration FIRST adds + // persistence (!prior). In every case the current route's islands must read + // the fields, seed and version they declared, with any saved value restored + // on top. Adopting persistence without re-seeding is unsafe: two routes can + // share top-level field names yet declare a different nested seed, and on a + // fresh storage slot (nothing to restore) the later route's islands would + // otherwise mount on the earlier route's seed. Divergent shapes that share a + // storage key are reported at build time by page_store_persist_key_conflict; + // a conflicting scope is kept first-wins (page_store_persist_scope_conflict), + // so navigation cannot thrash storage. const shapeChanged = !sameFieldSet(registry.fields[name] || [], fields) || - (hasPersist && prior && prior.version !== persist.version); + (hasPersist && (!prior || prior.version !== persist.version)); if (shapeChanged) { registry.fields[name] = fields; registry.seeds[name] = Object.assign(Object.create(null), seed); @@ -179,19 +184,6 @@ func storeRuntimeSource() string { notify(name); return; } - // Same shape: if this declaration adds persistence and we are not - // persisting yet, adopt it and restore the 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 (hasPersist && !prior) { - registry.persist[name] = persist; - const restored = readPersisted(persist, registry.fields[name]); - if (restored) { - registry.stores[name] = Object.assign({}, registry.stores[name] || {}, restored); - notify(name); - } - } return; } @@ -242,18 +234,27 @@ func storeRuntimeSource() string { }; }; - // 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") { + // Cross-tab sync: when another tab writes a persisted LOCAL store, mirror the + // value here and notify islands. Only localStorage is shared across tabs on the + // origin, so its "storage" event is what carries cross-tab writes. sessionStorage + // is partitioned per top-level tab, so session-scoped stores are deliberately + // tab-local and skipped here (a "storage" event for them only fires within the + // same page session, e.g. iframes). We never write back, so tabs cannot loop. + // + // Register exactly once per registry. SPA navigation can re-execute this script + // (the head swap drops the stores.js tag while window.__gowdkStores stays alive, + // so a later store page makes activateNewScripts treat it as new); a second + // listener would notify islands — and rerun render/effects — twice per write. + if (!registry.storageListenerAttached && typeof window.addEventListener === "function") { + registry.storageListenerAttached = true; window.addEventListener("storage", (event) => { if (!event || !event.key) return; Object.keys(registry.persist).forEach((name) => { const config = registry.persist[name]; - if (!config || config.key !== event.key) return; - // Local and session stores can share the gowdk:store: key. Ignore - // events from the other storage area so a localStorage write never - // overwrites a sessionStorage-backed store (or vice versa). Older - // browsers omit storageArea, where the key match alone is used. + if (!config || config.scope !== "local" || config.key !== event.key) return; + // Only local stores reach here, but keep the storageArea guard: a + // session store can share the gowdk:store: key, and older browsers + // omit storageArea, where the key + scope match alone is used. if (event.storageArea && event.storageArea !== storageFor(config.scope)) return; if (event.newValue == null) { if (registry.seeds[name]) registry.stores[name] = Object.assign({}, registry.seeds[name]); diff --git a/internal/buildgen/store_persist_runtime_test.go b/internal/buildgen/store_persist_runtime_test.go index 2d2d974c..e5d8e1d8 100644 --- a/internal/buildgen/store_persist_runtime_test.go +++ b/internal/buildgen/store_persist_runtime_test.go @@ -242,6 +242,51 @@ assert.equal(r.get("cart").Count, 1, "a storage event from a different area (ses storageListeners[0]({ key: "gowdk:store:cart", newValue: JSON.stringify({ v: "v1", s: { Count: 99, Open: true } }), storageArea: localStorage }); assert.equal(r.get("cart").Count, 99, "a storage event from the matching area is applied"); +// 12. Adopting persistence on a FRESH storage slot must re-seed to the later +// route's declared seed, not silently keep the earlier unpersisted route's seed. +// Two routes can share a top-level field name yet declare a different default; +// with nothing to restore, the current route's islands must read THEIR seed. +global.document.querySelectorAll = () => [{ + getAttribute(name) { return name === "data-gowdk-store" ? "banner" : null; }, + get textContent() { return '{"Dismissed":true}'; } +}]; +r.hydrate(); +assert.equal(r.get("banner").Dismissed, true, "precondition: unpersisted route uses its own seed"); +global.document.querySelectorAll = () => [{ + getAttribute(name) { + if (name === "data-gowdk-store") return "banner"; + if (name === "data-gowdk-persist") return "local"; + if (name === "data-gowdk-persist-key") return "gowdk:store:banner"; + if (name === "data-gowdk-persist-version") return "b1"; + return null; + }, + get textContent() { return '{"Dismissed":false}'; } +}]; +r.hydrate(); +assert.equal(r.get("banner").Dismissed, false, "adopting persistence on empty storage re-seeds to the persisted route's declared seed"); + +// 13. Session-scoped stores must NOT mirror cross-tab storage events: +// sessionStorage is partitioned per top-level tab, so a "storage" event from +// another tab cannot belong to this tab's session store. +seedJSON = '{"Count":0,"Open":false}'; +scope = "session"; +version = "v1"; +global.document.querySelectorAll = () => [makeNode()]; +r = boot(); +r.set("cart", { Count: 3, Open: false }); +storageListeners[0]({ key: "gowdk:store:cart", newValue: JSON.stringify({ v: "v1", s: { Count: 77, Open: true } }), storageArea: sessionStorage }); +assert.equal(r.get("cart").Count, 3, "a session-scoped store ignores cross-tab storage events"); + +// 14. Re-executing the runtime against the SAME window (as SPA navigation does +// when stores.js is re-activated) must NOT register a second storage listener; +// a duplicate would notify islands twice per cross-tab write. +scope = "local"; +global.document.querySelectorAll = () => [makeNode()]; +r = boot(); +assert.equal(storageListeners.length, 1, "one storage listener after boot"); +new Function(runtimeSrc)(); +assert.equal(storageListeners.length, 1, "re-executing the runtime does not add a second storage listener"); + console.log("OK"); ` } diff --git a/internal/clientrt/runtime.go b/internal/clientrt/runtime.go index 62558457..d2105eee 100644 --- a/internal/clientrt/runtime.go +++ b/internal/clientrt/runtime.go @@ -235,15 +235,43 @@ const runtimeSource = `(function () { } function activateNewScripts(previousScripts) { - var pending = []; + var storeScripts = []; + var otherScripts = []; Array.prototype.forEach.call(document.querySelectorAll('script[src]'), function (script) { if (previousScripts[script.src]) { return; } + if (script.hasAttribute('data-gowdk-store-runtime')) { + storeScripts.push(script); + } else { + otherScripts.push(script); + } + }); + // Run the store runtime first, then hydrate the registry, before island + // bundles execute. Island bundles auto-mount on execution and read + // window.__gowdkStores during mount, so a store loaded after them would leave + // islands mounted with no subscription or persisted value -- and the + // post-navigation mount pass skips already-mounted roots. Hydrating here also + // covers the case where stores.js already ran on the previous page (skipped + // above) yet this route introduces new store seeds. + return runScripts(storeScripts).then(function () { + if (typeof window !== 'undefined' && window.__gowdkStores && window.__gowdkStores.hydrate) { + window.__gowdkStores.hydrate(); + } + return runScripts(otherScripts); + }); + } + + function runScripts(scripts) { + var pending = []; + scripts.forEach(function (script) { var active = document.createElement('script'); Array.prototype.forEach.call(script.attributes, function (attr) { active.setAttribute(attr.name, attr.value); }); + // Dynamically inserted scripts default to async; force ordered execution so + // a dependency (the store runtime) cannot lose a race with its dependents. + active.async = false; pending.push(new Promise(function (resolve, reject) { active.onload = resolve; active.onerror = reject; diff --git a/internal/clientrt/runtime_test.go b/internal/clientrt/runtime_test.go index b9820a5c..6a8e8724 100644 --- a/internal/clientrt/runtime_test.go +++ b/internal/clientrt/runtime_test.go @@ -52,6 +52,14 @@ func TestSourceEmitsSPANavigationRuntime(t *testing.T) { `gowdk:navigate-error`, `window.location.href = url.href`, `activateNewScripts(previousScripts)`, + // The store runtime runs (and hydrates) before island bundles, which + // auto-mount on execution and read the store registry during mount. + `script.hasAttribute('data-gowdk-store-runtime')`, + `return runScripts(storeScripts).then(`, + `window.__gowdkStores.hydrate()`, + `return runScripts(otherScripts)`, + // Ordered (non-async) execution so a dependency cannot lose the race. + `active.async = false`, } { if !strings.Contains(source, expected) { t.Fatalf("expected runtime source to contain %q:\n%s", expected, source) diff --git a/internal/compiler/validate_page.go b/internal/compiler/validate_page.go index 3146402c..e77130fe 100644 --- a/internal/compiler/validate_page.go +++ b/internal/compiler/validate_page.go @@ -407,7 +407,10 @@ func validatePageStores(page gwdkir.Page) []ValidationError { // a field whose name resembles a secret earns a warning because browser storage // is readable by any script on the origin. func validateStorePersist(page gwdkir.Page, store gwdkir.Store, resolved gotypes.Struct) []ValidationError { - if store.Persist == "" { + // No `persist` clause: nothing to validate. An explicit but empty scope + // (`persist ""`) sets PersistSet, so it falls through to the scope check below + // and is reported as invalid rather than silently treated as unpersisted. + if !store.PersistSet { return nil } if store.Persist != "local" && store.Persist != "session" { diff --git a/internal/compiler/validate_page_persist_test.go b/internal/compiler/validate_page_persist_test.go index 29eee9f1..f283c14f 100644 --- a/internal/compiler/validate_page_persist_test.go +++ b/internal/compiler/validate_page_persist_test.go @@ -19,10 +19,11 @@ func persistedStorePageNamed(id, route, storeName, typeName, initName, scope str Path: "github.com/cssbruno/gowdk/testfixture/islands", }}, Stores: []gwdkir.Store{{ - Name: storeName, - Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, - Init: gwdkir.GoRef{Alias: "ui", Name: initName}, - Persist: scope, + Name: storeName, + Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, + Init: gwdkir.GoRef{Alias: "ui", Name: initName}, + Persist: scope, + PersistSet: scope != "", }}, Blocks: gwdkir.Blocks{View: true, ViewBody: `
Page
`}, } @@ -39,10 +40,11 @@ func persistedStorePage(storeName, typeName, initName, scope string) gwdkir.Page Path: "github.com/cssbruno/gowdk/testfixture/islands", }}, Stores: []gwdkir.Store{{ - Name: storeName, - Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, - Init: gwdkir.GoRef{Alias: "ui", Name: initName}, - Persist: scope, + Name: storeName, + Type: gwdkir.GoRef{Alias: "ui", Name: typeName}, + Init: gwdkir.GoRef{Alias: "ui", Name: initName}, + Persist: scope, + PersistSet: scope != "", }}, Blocks: gwdkir.Blocks{View: true, ViewBody: `
Cart
`}, } @@ -72,6 +74,22 @@ func TestValidatePageRejectsInvalidPersistScope(t *testing.T) { } } +func TestValidatePageRejectsEmptyPersistScope(t *testing.T) { + // `persist ""` (clause present, empty scope) must be diagnosed, not silently + // treated as unpersisted. The page builds the IR directly with PersistSet set + // and an empty scope, mirroring what the parser produces for `persist ""`. + page := persistedStorePage("cart", "CounterState", "NewCounterState", "") + page.Stores[0].PersistSet = true + diagnostics := ValidatePage(gowdk.Config{}, irPage(page)) + diagnostic := firstDiagnostic(diagnostics, "page_store_persist_scope_invalid") + if diagnostic == nil { + t.Fatalf("missing page_store_persist_scope_invalid diagnostic for empty scope: %#v", diagnostics) + } + if diagnostic.Severity != SeverityError { + t.Fatalf("empty persist scope should be an error, got severity %v", diagnostic.Severity) + } +} + func TestValidatePageWarnsOnPersistedSecretField(t *testing.T) { // SessionState has a Token field, which resembles a secret. page := persistedStorePage("session", "SessionState", "NewSessionState", "local") diff --git a/internal/gwdkast/ast.go b/internal/gwdkast/ast.go index 3ad83e5f..335d6b17 100644 --- a/internal/gwdkast/ast.go +++ b/internal/gwdkast/ast.go @@ -166,7 +166,12 @@ type Store struct { // scope value is validated later so an invalid literal still parses into a // store and yields a precise diagnostic rather than a generic parse error. Persist string - Span source.SourceSpan + // PersistSet reports whether a `persist` clause was present, distinguishing an + // absent clause from an explicit empty scope (`persist ""`). Without it the + // latter is indistinguishable from no persistence and would silently parse as + // unpersisted instead of yielding page_store_persist_scope_invalid. + PersistSet bool + Span source.SourceSpan } // StateContract describes a component state type and initializer. diff --git a/internal/gwdkir/ir.go b/internal/gwdkir/ir.go index c1f3ef8c..9a31fad3 100644 --- a/internal/gwdkir/ir.go +++ b/internal/gwdkir/ir.go @@ -84,7 +84,10 @@ type Store struct { // Persist is the optional `persist ""` modifier scope literal // ("" when not persisted). Validated in the compiler, not the parser. Persist string - Span source.SourceSpan + // PersistSet reports whether a `persist` clause was present, so an explicit + // empty scope (`persist ""`) is diagnosed instead of treated as unpersisted. + PersistSet bool + Span source.SourceSpan } // Page is the normalized IR for one page source. diff --git a/internal/parser/page_lower.go b/internal/parser/page_lower.go index dace7bfc..69ff5004 100644 --- a/internal/parser/page_lower.go +++ b/internal/parser/page_lower.go @@ -187,11 +187,12 @@ func lowerSyntaxStores(in []gwdkast.Store) []gwdkir.Store { out := make([]gwdkir.Store, 0, len(in)) for _, item := range in { out = append(out, gwdkir.Store{ - Name: item.Name, - Type: gwdkir.GoRef{Alias: item.Type.Alias, Name: item.Type.Name, Span: item.Type.Span}, - Init: gwdkir.GoRef{Alias: item.Init.Alias, Name: item.Init.Name, Span: item.Init.Span}, - Persist: item.Persist, - Span: item.Span, + Name: item.Name, + Type: gwdkir.GoRef{Alias: item.Type.Alias, Name: item.Type.Name, Span: item.Type.Span}, + Init: gwdkir.GoRef{Alias: item.Init.Alias, Name: item.Init.Name, Span: item.Init.Span}, + Persist: item.Persist, + PersistSet: item.PersistSet, + Span: item.Span, }) } return out diff --git a/internal/parser/page_test.go b/internal/parser/page_test.go index 1d2b353f..8905c83e 100644 --- a/internal/parser/page_test.go +++ b/internal/parser/page_test.go @@ -177,16 +177,20 @@ view { func TestParsePageReadsStorePersistModifier(t *testing.T) { for _, tc := range []struct { - name string - line string - scope string + name string + line string + scope string + persistSet bool }{ - {name: "local", line: `store cart ui.CounterState = ui.NewCounterState() persist "local"`, scope: "local"}, - {name: "session", line: `store cart ui.CounterState = ui.NewCounterState() persist "session"`, scope: "session"}, - {name: "no-persist", line: `store cart ui.CounterState = ui.NewCounterState()`, scope: ""}, + {name: "local", line: `store cart ui.CounterState = ui.NewCounterState() persist "local"`, scope: "local", persistSet: true}, + {name: "session", line: `store cart ui.CounterState = ui.NewCounterState() persist "session"`, scope: "session", persistSet: true}, + {name: "no-persist", line: `store cart ui.CounterState = ui.NewCounterState()`, scope: "", persistSet: false}, // An unknown scope still parses into a store so validation can emit a // precise diagnostic rather than a generic parse error. - {name: "invalid-scope", line: `store cart ui.CounterState = ui.NewCounterState() persist "disk"`, scope: "disk"}, + {name: "invalid-scope", line: `store cart ui.CounterState = ui.NewCounterState() persist "disk"`, scope: "disk", persistSet: true}, + // An explicit empty scope must be distinguishable from no persistence so it + // is diagnosed rather than silently treated as unpersisted. + {name: "empty-scope", line: `store cart ui.CounterState = ui.NewCounterState() persist ""`, scope: "", persistSet: true}, } { t.Run(tc.name, func(t *testing.T) { page, err := ParsePage([]byte("\npage cart\nroute \"/cart\"\n\nimport ui \"github.com/cssbruno/gowdk/testfixture/islands\"\n\n" + tc.line + "\n\nview {\n
Cart
\n}\n")) @@ -199,6 +203,9 @@ func TestParsePageReadsStorePersistModifier(t *testing.T) { if got := page.Stores[0].Persist; got != tc.scope { t.Fatalf("store persist = %q, want %q", got, tc.scope) } + if got := page.Stores[0].PersistSet; got != tc.persistSet { + t.Fatalf("store persistSet = %v, want %v", got, tc.persistSet) + } if page.Stores[0].Name != "cart" || page.Stores[0].Init.Name != "NewCounterState" { t.Fatalf("persist modifier corrupted the store: %#v", page.Stores[0]) } diff --git a/internal/parser/patterns.go b/internal/parser/patterns.go index 24ce7d13..af21e673 100644 --- a/internal/parser/patterns.go +++ b/internal/parser/patterns.go @@ -364,13 +364,17 @@ func parseStoreLine(line string) []string { return nil } persistScope := "" + persistSet := "" if len(tokens) == 8 { if tokens[6].Kind != syntax.TokenIdentifier || tokens[6].Lexeme != "persist" || tokens[7].Kind != syntax.TokenString { return nil } persistScope = syntax.Unquote(tokens[7].Lexeme) + // Record that the clause was present so an explicit `persist ""` is + // distinguishable from no persistence (both have an empty scope). + persistSet = "1" } - return []string{line, tokens[1].Lexeme, typeAlias, typeName, initAlias, initName, persistScope} + return []string{line, tokens[1].Lexeme, typeAlias, typeName, initAlias, initName, persistScope, persistSet} } func parseActionInputLine(line string) []string { diff --git a/internal/parser/syntax.go b/internal/parser/syntax.go index b47ee14a..a9a71061 100644 --- a/internal/parser/syntax.go +++ b/internal/parser/syntax.go @@ -252,11 +252,12 @@ func ParseSyntax(src []byte) (SyntaxFile, error) { if match := storePattern.FindStringSubmatch(line); match != nil { span := sourceLineSpan(lineNumber, rawLine) file.Stores = append(file.Stores, gwdkast.Store{ - Name: match[1], - Type: GoTypeRef{Alias: match[2], Name: match[3], Span: span}, - Init: GoFuncRef{Alias: match[4], Name: match[5], Span: span}, - Persist: match[6], - Span: span, + Name: match[1], + Type: GoTypeRef{Alias: match[2], Name: match[3], Span: span}, + Init: GoFuncRef{Alias: match[4], Name: match[5], Span: span}, + Persist: match[6], + PersistSet: len(match) > 7 && match[7] == "1", + Span: span, }) continue } diff --git a/internal/syntax/declparse.go b/internal/syntax/declparse.go index d465e9c9..abd194a1 100644 --- a/internal/syntax/declparse.go +++ b/internal/syntax/declparse.go @@ -190,6 +190,7 @@ func parseStoreTokens(src string, line []Token, end Token) (gwdkast.Store, bool) // An optional trailing `persist ""` clause bounds the initializer: the // init expression runs from `=` up to the `persist` keyword (or end of line). persistScope := "" + persistSet := false initStop := end.Offset for index := assign + 1; index < len(line); index++ { if line[index].Kind == TokenIdentifier && line[index].Lexeme == "persist" { @@ -197,6 +198,7 @@ func parseStoreTokens(src string, line []Token, end Token) (gwdkast.Store, bool) return gwdkast.Store{}, false } persistScope = Unquote(line[index+1].Lexeme) + persistSet = true initStop = line[index].Offset break } @@ -205,7 +207,7 @@ func parseStoreTokens(src string, line []Token, end Token) (gwdkast.Store, bool) if !ok { return gwdkast.Store{}, false } - return gwdkast.Store{Name: line[1].Lexeme, Type: typeRef, Init: initRef, Persist: persistScope, Span: span}, true + return gwdkast.Store{Name: line[1].Lexeme, Type: typeRef, Init: initRef, Persist: persistScope, PersistSet: persistSet, Span: span}, true } // parsePropsTokens recovers a `props ` contract: a type reference with From b520a7829ca61e02d97e7d3578c7308ff6adcbaa Mon Sep 17 00:00:00 2001 From: cssbruno Date: Sat, 13 Jun 2026 16:30:34 -0300 Subject: [PATCH 5/5] fix(stores): honor current-route persist and bulk storage clears - 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). --- internal/buildgen/runtime_assets.go | 25 ++++++++++++- .../buildgen/store_persist_runtime_test.go | 37 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/internal/buildgen/runtime_assets.go b/internal/buildgen/runtime_assets.go index 0631dd7f..ae0d4dd4 100644 --- a/internal/buildgen/runtime_assets.go +++ b/internal/buildgen/runtime_assets.go @@ -184,6 +184,14 @@ func storeRuntimeSource() string { notify(name); return; } + // Same field set and (if persisted) same version. If this route declares + // the store WITHOUT persistence but an earlier route persisted it, honor + // the current declaration and stop persisting, so set() does not keep + // writing to storage this route never opted into. The in-memory value is + // left intact so the store stays shared across routes. + if (!hasPersist && prior) { + delete registry.persist[name]; + } return; } @@ -248,7 +256,22 @@ func storeRuntimeSource() string { if (!registry.storageListenerAttached && typeof window.addEventListener === "function") { registry.storageListenerAttached = true; window.addEventListener("storage", (event) => { - if (!event || !event.key) return; + if (!event) return; + // A bulk Storage.clear() in another tab fires a "storage" event with a null + // key (and null newValue) rather than one event per removed key. Reset every + // local-scoped persisted store backed by the cleared area to its seed, so + // this tab does not keep values whose persisted backing is gone. Keyed + // setItem/removeItem (including __gowdkStores.clear) falls through below. + if (!event.key) { + if (event.storageArea && event.storageArea !== storageFor("local")) return; + Object.keys(registry.persist).forEach((name) => { + const config = registry.persist[name]; + if (!config || config.scope !== "local") return; + if (registry.seeds[name]) registry.stores[name] = Object.assign({}, registry.seeds[name]); + notify(name); + }); + return; + } Object.keys(registry.persist).forEach((name) => { const config = registry.persist[name]; if (!config || config.scope !== "local" || config.key !== event.key) return; diff --git a/internal/buildgen/store_persist_runtime_test.go b/internal/buildgen/store_persist_runtime_test.go index e5d8e1d8..1eb744e9 100644 --- a/internal/buildgen/store_persist_runtime_test.go +++ b/internal/buildgen/store_persist_runtime_test.go @@ -287,6 +287,43 @@ assert.equal(storageListeners.length, 1, "one storage listener after boot"); new Function(runtimeSrc)(); assert.equal(storageListeners.length, 1, "re-executing the runtime does not add a second storage listener"); +// 15. A route that declares the same store WITHOUT persist must stop an earlier +// route's persistence: after navigating to the unpersisted declaration, set() +// must not keep writing to storage, while the in-memory value stays shared. +seedJSON = '{"Count":0,"Open":false}'; +scope = "local"; +version = "v1"; +global.document.querySelectorAll = () => [makeNode()]; +r = boot(); +r.set("cart", { Count: 4, Open: false }); +assert.ok(localStorage.getItem("gowdk:store:cart") != null, "precondition: the persisted route writes storage"); +localStorage.removeItem("gowdk:store:cart"); +global.document.querySelectorAll = () => [{ + getAttribute(name) { return name === "data-gowdk-store" ? "cart" : null; }, + get textContent() { return '{"Count":0,"Open":false}'; } +}]; +r.hydrate(); +r.set("cart", { Count: 8, Open: true }); +assert.equal(localStorage.getItem("gowdk:store:cart"), null, "an unpersisted declaration stops set() from writing storage"); +assert.equal(r.get("cart").Count, 8, "the in-memory store stays shared across the navigation"); + +// 16. A bulk localStorage.clear() in another tab fires a "storage" event with a +// null key; local-scoped stores must reset to their seed instead of keeping +// values whose persisted backing is gone. A clear of the other area is ignored. +seedJSON = '{"Count":0,"Open":false}'; +scope = "local"; +version = "v1"; +global.document.querySelectorAll = () => [makeNode()]; +r = boot(); +r.set("cart", { Count: 6, Open: true }); +let bulkCleared = null; +r.subscribe("cart", (next) => { bulkCleared = next; }); +storageListeners[0]({ key: null, oldValue: null, newValue: null, storageArea: sessionStorage }); +assert.equal(r.get("cart").Count, 6, "a clear() of a different storage area is ignored"); +storageListeners[0]({ key: null, oldValue: null, newValue: null, storageArea: localStorage }); +assert.equal(r.get("cart").Count, 0, "a cross-tab localStorage.clear() resets local stores to seed"); +assert.ok(bulkCleared && bulkCleared.Count === 0, "a bulk clear notifies subscribers"); + console.log("OK"); ` }