diff --git a/CHANGELOG.md b/CHANGELOG.md index 99ece59..d6907de 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 8c1d3f8..7f2439e 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 684bacb..1989b81 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 fff663c..2adb4c9 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 de542d1..de8bf69 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 151393b..3146402 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 8ac089c..29eee9f 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 6b0172e..c30b711 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 e195632..ffcd186 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 b3d3518..0a725e0 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 6d1cf25..ba4e1f7 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"} }