feat(storage): add schema validation and serializers to defineItem#2484
Open
silverfish2525 wants to merge 35 commits into
Open
feat(storage): add schema validation and serializers to defineItem#2484silverfish2525 wants to merge 35 commits into
silverfish2525 wants to merge 35 commits into
Conversation
Introduces the type surface for issue wxt-dev#1173 (schema validation and serialization support for defineItem). No runtime behaviour change yet — new options are optional and untouched by the existing pipeline. Runtime plumbing follows in later commits. New public API: - WxtStorageItemOptions gains schema, serializer, onValidationError fields - WxtStorageItemSerializer<TValue, TRaw> with VueUse-verbatim read/write - OnValidationError<TValue> discriminant union with NoInfer-guarded callback - defineSchema<T>(parse) adapter that lifts any parse function into StandardSchemaV1<unknown, T> for TypeBox/io-ts/custom validators - Three new defineItem overloads placed ahead of the plain ones so TValue is driven by StandardSchemaV1.InferOutput<TSchema> whenever a schema is supplied Types follow the Golden Rule of Generics (every type parameter appears in at least two positions), use NoInfer<T> to keep the schema as the sole source of TValue, and rely on Standard Schema's own InferInput / InferOutput / Result / Issue types rather than re-declaring them. Deps added: @standard-schema/spec ^1.1.0 (types only, zero runtime), @standard-schema/utils ^0.3.0 (SchemaError; used in the read/write pipeline commits that follow). Tests: 9 new type-level assertions covering nullable/non-null schema inference, defineSchema sync + async parse wrapping, serializer read/write typing, onValidationError union coverage, and the NoInfer guarantee. All 240 tests pass. Refs: wxt-dev#1173
Step 2 of wxt-dev#1173. Adds the write pipeline; reads and watch still bypass both schema and serializer and land in follow-up commits. Pipeline in setValue: T \u2192 schema.validate \u2192 serializer.write \u2192 raw \u2192 driver.setItem Extracted into a top-level processWriteValue helper so the pipeline is swappable without touching the defineItem closure. The helper: - always throws on schema failure (writes ignore onValidationError \u2014 that option only affects reads and watch callbacks) - stores the schema's transformed output when the schema transforms (e.g. defineSchema(v => v * 2) stores 10 for setValue(5)) - runs serializer.write only after schema validation passes; a failed schema short-circuits and never calls the serializer - preserves the version-set bookkeeping already handled by setValue Tests: 8 new runtime tests across three describe blocks \u2014 schema-only writes, serializer-only writes, and the schema+serializer combination. Covers happy path, SchemaError shape, transformed-value storage, order of schema-then-serializer, and short-circuit on failed validation. Total 248 tests pass. Refs: wxt-dev#1173
Step 3 of wxt-dev#1173. Adds the read pipeline for defineItem.getValue on the non-init path. Init reads and watch callbacks still bypass the pipeline and land in follow-up commits. Pipeline in getValue (non-init path): raw \u2192 serializer.read? \u2192 schema.validate \u2192 T Extracted into a top-level processReadValue helper. Null raw short-circuits to fallback so validation never runs on the fallback sentinel. Non-null values always flow through the full pipeline. Schema failure invokes onValidationError: - 'throw' (default): SchemaError with the schema's issues array - 'fallback' : return fallback (or null) - 'reset' : driver.removeItem the invalid entry, return fallback - callback : return whatever the callback returns; not written back Migration order is preserved: migrations run against the raw wire form before the pipeline sees the value, so a migration can rescue a value that would otherwise fail the schema (test asserts this). Coerce-schema pattern is supported: with serializer.write set and no serializer.read, the raw goes straight to schema.validate; a coerce schema (e.g. z.coerce.date()) can turn the wire form into T. Tests: 13 new runtime tests across four describe blocks \u2014 schema reads, onValidationError strategies, serializer reads, and schema+serializer ordering. Total 261 tests pass. Refs: wxt-dev#1173
Step 4 of wxt-dev#1173. Adds the init pipeline for defineItem items that declare an `init` option. Watch callbacks still bypass both schema and serializer; that lands in the next commit. Two pipelines wire through getOrInitValue depending on storage state: storage empty : init() \u2192 processWriteValue \u2192 driver.setItem storage has value : driver.getItem \u2192 processReadValue \u2192 T processWriteValue now returns { raw, validated } so the init path can persist the raw wire form and return the validated runtime value to the caller without re-running validation. The order guard inside getOrInitValue is critical: items WITHOUT an init defined return the raw value unchanged and skip the pipeline. The non-init getValue path (Step 3) is the sole owner of the read pipeline for those items \u2014 running the pipeline again inside getOrInitValue would double-fire onValidationError callbacks against every eager migrationsDone.then(getOrInitValue). The eager migrationsDone.then(getOrInitValue) call now catches rejections so a schema failure on init doesn't produce an unhandled promise rejection at define-time. The same error still surfaces correctly when the user actually calls getValue \u2014 the mutex serialises both calls and the user gets an awaited error. Tests: 3 new runtime tests for the schema + init pairing \u2014 validator schema flowing through init writes and reads, SchemaError when init output fails validation (storage untouched), and preservation of existing storage on init items (init not re-run). Total 265 tests pass. Refs: wxt-dev#1173
Step 5 of wxt-dev#1173. The watch callback registered on a defineItem now receives the same T that getValue() would produce, not the raw wire form. Both the new and old values flow through processReadValue in parallel, so schema validation and serializer.read apply identically to reads and to change events. Failure modes align with getValue's read pipeline: - 'throw' (default): callback is skipped, pipeline error logged - 'fallback' : callback fires with fallback in place of invalid - 'reset' : storage cleared; a follow-up change event with null propagates through fallback - callback : user recovery result becomes the value passed to watch's callback The watch handler wraps the pipeline in try/catch: if either side throws ('throw' strategy), the user callback is not invoked and the error is console.error'd so silent failures don't hide bugs. Tests: 3 new runtime tests \u2014 serializer.read reaches watch callbacks,\n'throw' strategy skips the callback and logs, 'fallback' strategy\ndelivers the fallback to both slots. Total 268 tests pass.\n\nRefs: wxt-dev#1173
Step 6 of wxt-dev#1173. Adds a Schema Validation section to the WXT Storage guide covering: - Attaching a Standard Schema validator to defineItem (Zod, Valibot, ArkType, Effect Schema all work as-is) - Read and write pipeline diagrams so users know when validation and serialization run - The four onValidationError strategies for read-side recovery (throw / fallback / reset / callback) - A Custom Serialization subsection with the Set<string> and coerce-Date patterns - A Non-Standard-Schema Validators subsection for wrapping TypeBox, io-ts, or hand-rolled parsers via defineSchema Placed between Default Values and Bulk Operations in docs/storage.md. Refs: wxt-dev#1173
Cross-review by gpt-5.5 flagged one critical data-loss bug plus type
tightening and doc gaps. This commit addresses every Critical and
Should-fix item on the review.
Critical fixes:
- Watch + 'reset' data loss (packages/storage/src/index.ts:719).
When a valid newValue arrives alongside an invalid oldValue, the
oldValue's read pipeline was calling driver.removeItem, wiping the
just-written newValue. processReadValue now takes a pipelineOpts
argument with allowReset. The watch handler passes allowReset:false
on both sides, so watch never triggers a destructive removeItem
regardless of which slot fails. Direct getValue reads still honour
'reset'. Regression test added: setValue(5) with invalid oldValue -1
in storage now leaves storage as { count: 5 } instead of {}.
- Removed two 'as T' casts from processReadValue's schema and callback
paths. The schema branch now returns result.value directly (typed as
T by StandardSchemaV1.InferOutput). The callback branch relies on
narrowing after the string strategy checks. Only the trust-boundary
cast remains (no schema, no serializer) which mirrors the existing
typed getItem<T> helper contract.
Should-fix items:
- Added the missing schema + defaultValue overload for defineItem so
{ schema, defaultValue } drops the null just like { schema, fallback }
and { schema, init }. Type test added.
- Docs: fixed the defineSchema import example (was importing from
'#imports' which does not auto-import it; now imports from
'@wxt-dev/storage' and imports storage separately). Added a Caveats
section covering the transforming-schema round-trip footgun, that
bulk operations bypass schema and serializer, and that
onValidationError:'reset' is intentionally neutered inside watch
callbacks.
- Docs: replaced 'raw as string[]' in the serializer example with a
guarded new Set() that handles unexpected raw shapes.
- Added watch pipeline tests for the reset non-destructive case and the
callback recovery strategy.
- Vendor string updated to '@wxt-dev/storage/defineSchema' so Standard
Schema tooling attributing errors to a schema can point back to the
actual package name.
Tests: 271 total, 3 new (reset non-destructive, callback recovery,
schema+defaultValue type). All checks pass: Oxlint, Publint, TypeScript.
Refs: wxt-dev#1173
Turns on every strictness flag TypeScript offers beyond the base
`strict: true`, scoped to the `@wxt-dev/storage` package's tsconfig:
- noUncheckedIndexedAccess arr[0] is now T | undefined
- exactOptionalPropertyTypes foo?: T is not foo: T | undefined
- noImplicitOverride override keyword required on class overrides
- noImplicitReturns every code path must return
- noFallthroughCasesInSwitch switch cases must break / return
- noPropertyAccessFromIndexSignature bracket access for index signatures
- noUncheckedSideEffectImports TS 5.6+ side-effect import guard
- allowUnreachableCode: false dead code is an error
- allowUnusedLabels: false stray labels are an error
Fixes triggered by the new checks:
- WxtStorageItemOptions.fallback and .defaultValue now typed as
`T | undefined` (was `T` under `?:`). Under exactOptionalPropertyTypes
a bare `?:` means "the field may be absent", NOT "may be present as
undefined". Existing tests + user code that pass fallback: undefined
explicitly stay valid.
- Same widening on GetItemOptions.
- migrate()'s driver.getItems([...]) destructure replaced with
positional access + optional chaining. The pair-shape is guaranteed
by driver contract but not by the type; noUncheckedIndexedAccess
correctly rejects the old destructure.
- Two test-side arr[0] accesses guarded with optional chaining / defined
check now that the index type is T | undefined.
No public API shape changes. All 271 tests pass, TypeScript + Oxlint +
Publint clean. This is a pure strictness-tightening commit \u2014 no runtime
behaviour change.
Follow-up plans (out of scope; documented in
`advisor-plans/type-audit-storage/`):
002 kill 7 caller-invented generics on WxtStorage public API
(semver-major, deferred).
003 tighten watch listener change payload.
004 as const satisfies on drivers + drop the single non-null !.
Refs: wxt-dev#1173
… stack
Push key-literal precision through the storage package's type surface so
that `defineItem('local:theme', ...).key` narrows from
`StorageItemKey` to the exact literal `'local:theme'` at the call site.
Interface changes:
- `WxtStorageItem<TValue, TMetadata, TKey extends StorageItemKey =
StorageItemKey>` gains a third type parameter for the storage key. The
default is `StorageItemKey` so existing user code that spells the type
out as `WxtStorageItem<T, M>` continues to compile unchanged.
- Every `defineItem` overload (9 total: 1 key-only, 4 schema-carrying,
4 non-schema) grows a trailing `const TKey extends StorageItemKey =
StorageItemKey` type parameter. TypeScript infers `TKey` as the
literal string when the caller relies on full inference; it defaults
to the wide `StorageItemKey` union when the caller supplies any
explicit generic argument (`defineItem<number>('local:test', {})`).
This is a TypeScript limitation \u2014 `const T = default` type parameters
do not run inference under partial explicit generics \u2014 and is
documented via a new type test.
Internal-helper precision:
- `getMetaKey<const K extends string>(key: K): \`${K}$\`` \u2014 the meta-key
suffix is now visible at the type level. Previously it was
`(string) => string`.
- `resolveKey<const K extends StorageItemKey>(key: K): ResolvedKey<K>`
where `ResolvedKey<K>` parses the template literal via `K extends
\`${infer A extends StorageArea}:${infer Rest}\``. Callers now see
`driverArea` narrowed to the specific `StorageArea` variant and
`driverKey` narrowed to the trailing literal. Kept internal (not
exported) because it references the internal `WxtStorageDriver`.
- `MetaKey<K>` is exported so consumers can compose meta-key types.
New type tests (3):
- `MetaKey<'theme'> === 'theme$'` plus a degrade-to-`\`${string}$\``
case when the input is the bare `string` type.
- `defineItem('local:theme', { fallback: 5 }).key` narrows to
`'local:theme'` \u2014 the happy path.
- `defineItem<number>('local:test', {}).key` widens to
`StorageItemKey` \u2014 the explicit-generic escape hatch, documented so
future readers don't file it as a regression.
Not touched (documented in `advisor-plans/type-audit-storage/002-*.md`
as follow-ups, all semver-major):
- `getItem<T>`, `getMeta<T>`, `watch<T>`, `setItem<T>`,
`setMeta<T>` all carry caller-invented generics that violate the
Generics Golden Rule (T appears only in return / only in one param).
Removing them is a breaking change; deferred.
271 pre-existing tests still pass. 3 new type-level tests bring the
total to 274. Oxlint, Publint, TypeScript checks all clean.
Refs: wxt-dev#1173
…rc/types.ts
Phase A of the type-safety uplift. Splits the storage package's type
declarations into a foundation file so later phases (batch-API tuple
inference, migrations chain inference, unknown-boundary sweep) have a
canonical location to add typed utilities.
New file `packages/storage/src/types.ts`:
- `StorageArea` / `StorageItemKey` primitives.
- `MetaKey<K>` template-literal helper.
- `KeyParts<K>` \u2014 pure template-literal parse of a StorageItemKey into
`{ readonly driverArea; readonly driverKey }`. Extracted from the old
`ResolvedKey<K>` so it can be composed without dragging the
`WxtStorageDriver` runtime interface along.
- `WatchCallback`, `Unwatch`, `StorageAreaChanges` (`readonly` index
signature).
- `WxtStorageItemSerializer`, `OnValidationError`,
`WxtStorageItemOptions`.
- `GetItemOptions`, `RemoveItemOptions`, `SnapshotOptions`
(`excludeKeys: readonly string[]`).
- `NullablePartial`.
`index.ts` changes:
- Imports the primitives from `./types` and re-exports every public one
so consumers of `@wxt-dev/storage` see no surface change.
- Replaces the old inline `ResolvedKey<K>` conditional with
`type ResolvedKey<K extends StorageItemKey> = KeyParts<K> & {\n driver: WxtStorageDriver }`.
Stays private because `WxtStorageDriver` is internal.
- Drops the now-unused `Browser` import (`StorageAreaChanges` moved).
- `resolveKey` uses `as unknown as ResolvedKey<K>` for the trust-boundary
cast \u2014 `KeyParts<K>` has `readonly` fields that TS's overlap check
treats as non-assignable from the mutable object literal, and the
double-cast makes the trust boundary explicit at the call site.
Every user-facing type name and export path is preserved. 274 tests
still pass. Oxlint / Publint / TypeScript all clean.
Refs: wxt-dev#1173
…rnals
Phase B of the type-safety uplift. Turns every locally-fixable `any` into
`unknown` (or a precise `Record<string, unknown>`), tightens 4 driver
interface signatures, and eliminates 5 avoidable `as` casts. Every cast
that remains is now a documented trust boundary.
Internal helpers (were `any`, now precise):
- `mergeMeta(old, new): Record<string, unknown>` — both inputs typed.
- `getValueOrFallback<T>(value: T | null | undefined, fallback: T | null | undefined): T | null` — one generic drives both args and the return.
- `getMetaValue(properties: unknown): Record<string, unknown>` — the `typeof === 'object'` guard now honoured at the type level.
- `getItem<T>` helper is generic over T instead of `GetItemOptions<any>`.
- `getMeta` returns `Promise<Record<string, unknown>>` internally; the cast to caller-invented T is confined to the interface impl with an inline trust-boundary comment (Phase D removes it entirely).
- `setItem(driver, key, value: unknown)` replaces `value: any`.
- `setMeta(driver, key, properties: unknown)` accepts `unknown` and coerces through `getMetaValue`.
- `watch<T>(driver, key, cb: WatchCallback<T | null>)` — was `WatchCallback<any>`, now generic.
Batch buckets (were `any`, now `unknown`):
- `Map<string, GetItemOptions<unknown> | undefined>`.
- `Map<StorageItemKey, unknown>`.
- `Record<string, unknown>` in `getItems`.
- `Record<StorageArea, Array<{ key: string; value: unknown }>>` for fan-out.
- `Record<StorageArea, { key: string; properties: unknown }[]>` for `setMetas` fan-out.
- `getStorageArea().get<Record<string, unknown>>(key)` driver call.
Driver interface (`WxtStorageDriver`) tightening:
- `getItem<T>(key): Promise<T | null>` — impl now returns `(res[key] as T | null) ?? null` with a trust-boundary comment. The `<T>` stays until Phase D deletes caller-invented generics.
- `getItems(keys: readonly string[]): Promise<Array<{ key: string; value: unknown }>>` — was mutable and typed `any`.
- `setItems(values: ReadonlyArray<{ key: string; value: unknown }>): Promise<void>` — same.
- `restoreSnapshot(base: StorageArea, data: Record<string, unknown>): Promise<void>` — was `data: any`.
Type-erasure fixes without new casts:
- New `typedEntries<K, V>(obj: Partial<Record<K, V>>): Array<[K, V]>` helper. Preserves the key type across `Object.entries`. Kills 4 per-loop `as StorageArea` casts in `getMetas` / `setItems` / `setMetas` / `removeItems` fan-out.
- Template-literal type flow: the join of `driverArea: StorageArea` and `driverResult.key: string` is now assigned directly to `StorageItemKey` without a cast — TS narrows the joined literal automatically.
Storage-change payload:
- `{ newValue?: unknown; oldValue?: unknown }` — was `any`. Callback cast to `T | null` at the boundary is documented as a Phase-D-flagged Golden-Rule violation on `WxtStorage.watch<T>`.
Trust-boundary casts that survive (each with inline comment):
- `key.substring(...) as StorageArea` in `resolveKey` — runtime `substring` can't be proven identical to a compile-time literal.
- `as unknown as ResolvedKey<K>` at end of `resolveKey` — same reason, plus `readonly` fields on `KeyParts<K>`.
- `raw as T` in `processReadValue`'s no-schema-no-serializer branch — the only trust hole users opt into by skipping schema.
- `res[key] as T | null` in driver `getItem` impl — Phase D kills.
- `(change.newValue ?? null) as T | null` in watch impl — Phase D kills.
- `(await getMeta(...)) as T` in `getMeta` interface impl — Phase D kills.
- `properties as Record<string, unknown>` inside `getMetaValue` — narrowed by the `typeof === 'object'` guard, safe.
- `WxtStorageItem<any, any>` impl signatures — TS language requirement for overloaded function impl signatures.
`any` count in `packages/storage/src/index.ts` drops from 32 to 14. All 14 survivors are on `WxtStorage` / `WxtStorageDriver` public interface method signatures that Phase D (kill caller-invented generics) will replace with `unknown`.
274 tests pass. Oxlint / Publint / TypeScript all clean under the 9 strictest flags enabled in `9c1874a5`.
Refs: wxt-dev#1173
…eline typing
Phase C of the type-safety uplift. Threads the serializer's wire-form
type parameter (`TRaw`) through `WxtStorageItemOptions` and every
`defineItem` overload so that `serializer.read`'s `raw` parameter is
narrowed to whatever `serializer.write` actually returns. Previously
`raw` was always `unknown` and users had to write `raw as string[]`
inside `read` to recover the type.
Type-surface changes:
- `WxtStorageItemOptions<T, TRaw = unknown>` — new second generic. The
`serializer` field now uses `WxtStorageItemSerializer<T, TRaw>`
instead of `WxtStorageItemSerializer<T>` (which had TRaw stuck at
its default).
- All 8 `defineItem` overloads that take an `options` parameter grow a
trailing `TRaw = unknown` generic and pass it to
`WxtStorageItemOptions<..., TRaw>`. The key-only overload doesn't
take options, so no TRaw generic there — Oxlint would flag it as
unused.
- Schema-carrying overloads pass TRaw the same way; TSchema drives
TValue, TRaw is independent (schema validates the runtime value
after `read` runs, so it operates on `TValue`, not `TRaw`).
Inference behaviour (documented on the `serializer` JSDoc):
- Works when TS can run full inference on `defineItem`:
* `write`'s parameter is annotated:
`write: (set: Set<string>) => [...set]` → TValue=Set<string>,
TRaw=string[], `read`'s raw: string[].
* Or `fallback` / `defaultValue` is typed and drives TValue.
- Does NOT work when caller supplies an explicit
`defineItem<TValue>(...)` generic — TS's algorithm commits TRaw to
its `unknown` default before looking at `write`'s return type. A
standalone reproduction (in a 4-generic function with `TRaw =
unknown`, `TMeta = {}`, `const TKey = ...`) confirms this is a TS
language limitation, not a design flaw. Documented so users know
which pattern gives the best types.
Test changes:
- Existing "runs serializer.read on the raw storage value" test:
dropped the `raw as string[]` user-facing cast. `read: (raw) => new
Set(raw)` now type-checks cleanly because `raw` is inferred as
`string[]` from `write`'s inferred return type.
- New test "infers TRaw from serializer.write return type (fallback
path)" documents the fallback-driven inference path with an
`expectTypeOf` assertion.
275 tests pass (up from 274). Oxlint / Publint / TypeScript all clean
under the 9 strictest flags.
Refs: wxt-dev#1173
…r APIs
Phase D of the type-safety uplift. Removes every Generics-Golden-Rule
violation on `WxtStorage` and `WxtStorageDriver` — generics that
appeared only in a return type or only in a single parameter and were
therefore silent assertions dressed up as typed generics.
## SEMVER-MAJOR — breaking API surface changes
Public API (`WxtStorage`):
- `getItem<T>(key): Promise<T | null>` → `getItem(key): Promise<unknown>`.
The with-fallback overload keeps its `<TValue>` generic (Golden-Rule
correct — TValue drives both `fallback` and the return type). Users
who wrote `storage.getItem<Foo>('key')` must now either add a
`fallback` or narrow the returned `unknown` at the call site (schema,
runtime guard, or explicit assertion).
- `getMeta<T extends Record<string, unknown>>(key): Promise<T>` →
`getMeta(key): Promise<Record<string, unknown>>`. Metadata is
arbitrary at the storage layer; users narrow at the call site.
- `setItem<T>(key, value: T | null)` → `setItem(key, value: unknown)`.
- `setMeta<T extends Record<string, unknown>>(key, properties: T | null)`
→ `setMeta(key, properties: Record<string, unknown> | null)`.
- `watch<T>(key, cb: WatchCallback<T | null>)` →
`watch(key, cb: WatchCallback<unknown>)`. Watch callbacks now receive
raw wire data typed `unknown`; users who need narrow types define a
`WxtStorageItem` with a `schema` and use `item.watch(cb)`, which runs
the read pipeline and delivers typed values.
- `getItems`, `setItems`, `getMetas`, `setMetas`: bucket types tighten
from `any` to `unknown` / `Record<string, unknown>`; input arrays are
now `readonly`.
Internal API (`WxtStorageDriver`):
- `getItem<T>(key): Promise<T | null>` → `getItem(key): Promise<unknown>`.
- `setItem<T>(key, value: T | null)` → `setItem(key, value: unknown)`.
- `watch<T>(key, cb: WatchCallback<T | null>)` →
`watch(key, cb: WatchCallback<unknown>)`.
- `removeItems(keys: string[])` → `removeItems(keys: readonly string[])`.
## Implementation changes
Trust-boundary casts eliminated by the interface tightening:
- `res[key] as T | null` in driver `getItem` impl — gone; the return
type IS `unknown`.
- `(change.newValue ?? null) as T | null` and its `oldValue` sibling in
driver `watch` impl — gone; callback expects `unknown`.
- `(await getMeta(...)) as T` in `WxtStorage.getMeta` impl — gone; the
helper's `Record<string, unknown>` return type is now the interface
contract.
Internal `watch<T>(driver, key, cb)` helper keeps its generic (callers
provide narrow callbacks tied to their `defineItem`'s `TValue`) and
casts the callback to `WatchCallback<unknown>` at the driver boundary
— documented as safe because the schema-driven read pipeline validates
before invoking the callback.
`WxtStorage.getItem` implementation is written as a generic arrow and
cast to `WxtStorage['getItem']` to satisfy the two-overload interface
(with-fallback + without-fallback) that a plain arrow can't express.
`browser.storage.*.remove` accepts a mutable `string | number | (string
| number)[]`; the driver's `readonly string[]` is copied at the
boundary rather than widening the public contract.
## Test changes
- Existing test "should return a nullable type when getItem is called
without a fallback" — updated to "should return `unknown` when
getItem is called without a fallback" with an `expectTypeOf(res)
.toEqualTypeOf<unknown>()` assertion.
- New type-level tests:
* `storage.setItem` param is `unknown`.
* `storage.watch` callback is `WatchCallback<unknown>`.
* `storage.getMeta` return is `Record<string, unknown>`.
## Cast inventory delta
`as T` casts in `packages/storage/src/index.ts` drop from 7 to 4:
- `res as T | null | undefined` in internal `getItem` helper — the T is
honestly driven by `opts.fallback` when present.
- `raw as T` in `processReadValue` no-schema-no-serializer branch —
documented trust hole users opt into.
- `key.substring(...) as StorageArea` + `as unknown as ResolvedKey<K>`
in `resolveKey` — TS language limitation (runtime substring can't be
proven identical to a compile-time template-literal parse).
278 tests pass (up from 275, +3 Phase D type-level tests). Oxlint /
Publint / TypeScript all clean under the 9 strictest flags.
Refs: wxt-dev#1173
Phase E of the type-safety uplift. Rewrites the batch-read APIs so the
returned tuple preserves each input element's literal key type and, when
a `WxtStorageItem` is passed, threads its `TValue` into `result[i].value`.
Previously:
const [a, b] = await storage.getItems(['local:x', myItem]);
// a: { key: StorageItemKey; value: unknown } — key type erased
// b: { key: StorageItemKey; value: unknown } — TValue lost
Now:
const [a, b] = await storage.getItems(['local:x', myItem] as const);
// a: { readonly key: 'local:x'; readonly value: unknown }
// b: { readonly key: 'local:theme'; readonly value: Theme }
New type primitives in `types.ts`:
- `WxtStorageItemLike<TValue, TKey>` — structural forward-declaration of
the runtime `WxtStorageItem` interface, carrying `key`, `getValue`, and
`fallback`. Kept in `types.ts` so the batch mapped tuples stay free of
runtime-driver dependencies.
- `GetItemsInputElement` — union of the three shapes a `getItems` input
can take: bare `StorageItemKey`, `WxtStorageItemLike`, or
`{ key; options? }` bag.
- `GetItemsResult<T>` — mapped tuple. Per element:
* `WxtStorageItem<V, _, K>` → `{ key: K; value: V }`.
* `{ key: K; options?: GetItemOptions<V> }` → `{ key: K; value: V | null }`.
* bare literal key `'local:x'` → `{ key: 'local:x'; value: unknown }`.
* wider `StorageItemKey` union → `{ key: StorageItemKey; value: unknown }`.
- `GetMetasInputElement`, `GetMetasResult<T>` — same pattern for `getMetas`,
though `meta` stays `Record<string, unknown>` because the storage layer
doesn't type metadata.
Interface method changes:
- `getItems<const T extends ReadonlyArray<GetItemsInputElement>>(keys: T): Promise<GetItemsResult<T>>`.
The `const T` type parameter preserves tuple shape; users pass `[...] as const`
to hold literal keys narrow.
- `getMetas<const T extends ReadonlyArray<GetMetasInputElement>>(keys: T): Promise<GetMetasResult<T>>`.
Implementation:
- Both impls keep their runtime shape (return `{key, value}[]` /
`{key, meta}[]`) and cast to `WxtStorage['getItems']` /
`WxtStorage['getMetas']` at the return boundary. The runtime doesn't
know or care about the mapped-tuple type — it's a compile-time-only
narrowing over the input tuple.
Public exports:
- `GetItemsInputElement`, `GetItemsResult`, `GetMetasInputElement`,
`GetMetasResult` are re-exported from `./index.ts` for consumers that
want to type their own wrappers.
Type-level tests (2 new):
- `getItems(['local:a', 'session:b', 'sync:c'] as const)` — asserts
each `result[i].key` narrows to the exact literal string, and
`result[i].value` is `unknown`.
- `getItems([numItem, strItem] as const)` — asserts full inference:
keys are literals, values match each `defineItem`'s TValue.
280 tests pass (up from 278, +2 Phase E). Oxlint / Publint / TypeScript
all clean under the 9 strictest flags.
Refs: wxt-dev#1173
…elper
Phase F of the type-safety uplift. Replaces the untyped
`Record<number, (any) => any>` migrations map with a positional tuple
and adds a `defineMigrations<TValue>()` helper that validates each
migration in the chain at the type level.
## SEMVER-MAJOR — breaking migrations API
Before:
migrations: {
2: (old) => old * 2,
3: (old) => old + 10,
}
After (basic form — untyped chain):
migrations: [
(old) => (old as number) * 2, // v1 → v2
(old) => (old as number) + 10, // v2 → v3
]
After (chain-checked via helper):
migrations: defineMigrations<number>()(
(v) => Number(v) * 2, // v1 → v2 (returns number)
(v) => v + 10, // v2 → v3 (v is number, must return number = TValue)
)
Position `i` (0-indexed) migrates from version `i + 1` to version `i +
2`. `defineMigrations` is an overload family up to 6 chained versions;
beyond that, a fallback overload accepts an untyped variadic array.
Each overload's final generic is constrained `extends TValue`, so the
last migration's return type must match the item's TValue.
## Alternatives documented in JSDoc
Per user request, the JSDoc on `WxtStorageItemOptions.migrations` lists
three alternate designs so aklinker can pick a different one on review:
- B. `defineMigrations<TValue>({ 2: fn, 3: fn })` builder with per-
version types. Keeps arbitrary numeric keys.
- C. `Record<number, (oldValue: unknown) => unknown>`. Kills `any` but
leaves the chain unchecked. Smallest breaking change.
- D. Keep the current `Record<number, (any) => any>`. Zero-risk.
## Runtime change
`migrations?.[migrateToVersion]` becomes `migrations?.[migrateToVersion
- 2]`. `migrationsToRun` iterates version numbers starting at 2;
position 0 in the array corresponds to the v1 → v2 migration, so the
translation is `version - 2`.
## Test changes
- 14 test-site migrations converted from `{ N: fn }` object literal to
`[fn, ...]` tuple form.
- 2 `vi.fn((n: number) => n * 2)` mocks tightened to `(n: unknown) =>
(n as number) * 2` so they assign cleanly to the new
`(oldValue: unknown) => unknown` element type (contravariance
requires the wider `unknown` param on the callback shape).
- 1 in-line migration `(old: number) => Math.abs(old)` rewritten to
`(old: unknown) => Math.abs(old as number)` for the same reason.
280 tests pass. Oxlint / Publint / TypeScript all clean under the 9
strictest flags. No new tests were added (per user direction — the
runtime test suite already covers the migration behaviour end-to-end;
the chain-check happens at compile time and would only add duplicative
type-level assertions).
Refs: wxt-dev#1173
Cross-review at Phase F exit (gpt-5.5/high) returned REJECT with 4
blocking issues + 1 nice-to-have. All addressed.
## Blocking-1: `resolveKey` banned `as unknown as ResolvedKey<K>`
`KeyParts<K>` was expressed as a top-level distributive conditional
`K extends ... ? {readonly driverArea; readonly driverKey} : never`,
which TS treats as opaque when K is generic. Fresh object literals
built from narrowed strings could not be cast to it without going
through `unknown`.
Rewritten as a mapped-style structural type:
export type KeyParts<K extends StorageItemKey> = {
readonly driverArea: K extends `${infer A extends StorageArea}:${string}` ? A : never;
readonly driverKey: K extends `${StorageArea}:${infer R}` ? R : never;
};
Each field carries the nested conditional, so field-level assignment
from a narrowed `string` succeeds and the return value casts to
`ResolvedKey<K>` with a single `as` — no `as unknown as`.
## Blocking-2: `defineMigrations` output not assignable to `migrations`
Chained helper returns `readonly [(v: unknown) => A, (v: A) => B, ...]`.
The prior option type `ReadonlyArray<(oldValue: unknown) => unknown>`
rejected narrow-param fns because parameters are contravariant under
`strictFunctionTypes`.
Option type widened to:
migrations?: ReadonlyArray<(oldValue: any) => unknown | Promise<unknown>>;
`any` in the parameter position is intentional: the chain is
heterogeneous by design (each fn accepts the previous fn's return
type). The `defineMigrations` helper enforces the actual chain typing
at construction; the option type only needs to accept the resulting
tuple. Trust boundary documented in JSDoc + inline eslint-disable.
Verified against a standalone smoke probe:
storage.defineItem<number>('local:count', {
version: 3, fallback: 0,
migrations: defineMigrations<number>()(
(v) => Number(v) * 2, // v1 → v2
(v) => v + 10, // v2 → v3
),
});
Type-checks clean.
## Blocking-3: untrusted `meta.v` cast to number
Previous code: `const meta = (results[1]?.value ?? {}) as {v?: number; ...}`
then `const currentVersion = meta?.v ?? 1;`.
A corrupted browser profile holding `{v: "bad"}` under `${key}$`
would make `targetVersion - currentVersion` produce `NaN`, skip the
migration loop silently, and then overwrite the metadata with a valid
`{v: targetVersion}` — masking corruption without running migrations.
Narrowed at runtime:
const rawMetaValue = results[1]?.value;
const meta: Record<string, unknown> =
rawMetaValue != null &&
typeof rawMetaValue === 'object' &&
!Array.isArray(rawMetaValue)
? (rawMetaValue as Record<string, unknown>)
: {};
const rawV = meta['v'];
const storedVersion: number | undefined =
typeof rawV === 'number' && Number.isInteger(rawV) && rawV >= 1
? rawV
: undefined;
`storedVersion == null` is now the single source of truth for
`needsVersionSet`. `currentVersion` falls back to 1 only when the
stored `v` is genuinely a valid positive integer that we can trust.
## Blocking-4: `getItems` duplicate-key overwrote fallbacks
Previous impl kept a `Map<string, opts>` keyed by the resolved key
string. If the input tuple contained the same key twice with different
options, the second call to `keyToOptsMap.set(keyStr, opts)` overwrote
the first, and both result slots picked up the *second* slot's
fallback. This directly contradicted the per-slot mapped-tuple result
typing added in Phase E.
Restructured to a per-slot array. Each input element pushes a `Slot =
{key; opts}`. Fetches still batch by driver area (dedup happens
implicitly via `driver.getItems`), and the final `slots.map(...)` uses
each slot's own `opts` — duplicate keys with different fallbacks now
return each slot's own resolved value.
## Nice-to-have: `WxtStorageItemLike<any, infer K>` → `<unknown, infer K>`
`any` had no variance justification. Replaced with `unknown` inside
the `GetMetasResult` mapped type.
## Not addressed (intentional, per user direction)
Reviewer asked for compile-time tests around `defineMigrations` and
`{ key, options: { defaultValue } }` inputs. The user has explicitly
paused new type-level tests for this branch; runtime tests already
cover the migration behaviour end-to-end, and the smoke probe above
verified the Phase F helper integration.
280 tests pass. `bun run check` green.
Adds committed regression coverage for the two behaviours the
re-review flagged as untested. All 4 code-level blockers were resolved
in `4b7db583`; the outstanding CHANGES_REQUESTED was gated on this
coverage.
## 1. getItems duplicate-key different-fallback slots
`packages/storage/src/__tests__/index.test.ts` — new case under
`describe('getItems')`:
const result = await storage.getItems([
{ key: 'local:dup', options: { fallback: 'alpha' } },
{ key: 'local:dup', options: { fallback: 'beta' } },
]);
expect(result).toEqual([
{ key: 'local:dup', value: 'alpha' },
{ key: 'local:dup', value: 'beta' },
]);
Pins the per-slot options behaviour. Fails against the pre-`4b7db583`
`Map<key, opts>` impl (both slots would return `'beta'`).
## 2. Malformed meta.v narrowing
Parameterized `it.each` under `describe('versioning')` — one case per
malformed shape:
- string `'bad'`
- `Number.NaN`
- negative `-3`
- non-integer `1.5`
- zero `0`
Each case seeds `count$` with the malformed `v`, defines an item at
version 3 with two migrations, and asserts:
- Both migrations run (currentVersion falls back to 1).
- The value ends at `12` (`2 * 2 * 3`).
- Metadata ends at `{ v: 3 }` — corruption is overwritten with a
valid version.
Fails against the pre-`4b7db583` `meta.v as number` cast (migrations
would skip silently when arithmetic yielded NaN or when negative /
non-integer values were used as-is).
286 tests pass (up from 280). `bun run check` clean.
Reviewer flagged the two `(oldCount as number)` assertions in the
parameterized malformed-meta.v test as the repo-banned sloppenheimer
cast pattern. Fixed by relying on vi.fn's default `Procedure` context
(`(...args: any[]) => any`), which contextualizes lambda parameters as
`any` without requiring explicit narrowing or type assertions:
const migrateToV2 = vi.fn((oldCount) => oldCount * 2);
Also removed 3 pre-existing `(n as number)` and `(old as number)`
casts introduced during Phase F when the `migrations` option type was
tightened. Those tests now use the same inferred-any style as the
rest of the migration test suite.
286 tests pass. `bun run check` clean.
…const K>
Deep audit of the recent `<G extends string>` propagation caught 9 concrete
shortcomings. Fixed by moving from class-level G to per-method `<const K
extends StorageItemKey>`, which is strictly stronger.
## Why the class-level G was wrong
`createStorage<G>()`, `WxtStorage<G>`, `WxtStorageDriver<G>`, and
`createDriver<G>` all attached G to the *instance*, but "which key am I
looking at" is a *per-call* fact. The exported singleton `storage =
createStorage()` was collapsing to `WxtStorage<string>` (default), giving
zero narrowing at the call site.
Per-instance G also broke Phase E: `getItems<G>(keys:
ReadonlyArray<StorageItemKey<G>>)` forced every element to share the same
G, so `['local:a', 'sync:b']` unified G to `'a' | 'b'` — a shared union
instead of per-element narrowing. Kills mapped-tuple inference.
## Nine shortcomings, all addressed
1. `WxtStorage<G extends string>` class generic → removed (singleton was
collapsing to `<string>`).
2. `WxtStorageDriver<G>` class generic → removed. Drivers operate on
bare keys after `resolveKey` strips the area prefix; G at driver level
narrowed nothing anything downstream used.
3. `createStorage<G>()` / `createDriver<G>()` → removed.
4. `KeyParts<G, K extends StorageItemKey<G>>` → `KeyParts<K extends
StorageItemKey>`. G was derivable from K; single-parameter form
guarantees `driverArea` and `driverKey` stay correlated.
5. `WxtStorageItem<G, TValue, TMetadata, TKey>` → `WxtStorageItem<TValue,
TMetadata, TKey>`. TKey already captures the specific key literal;
G duplicated that.
6. `getItems<G>(keys: Array<StorageItemKey<G>>)` → `getItems<const T
extends ReadonlyArray<GetItemsInputElement>>(keys: T)`. Phase E
mapped-tuple restored: `[a, b]` binds T to a 2-tuple, each slot
narrowed independently.
7. Line 375 `as StorageItemKey<G>` cast → removed. Bare
`${StorageArea}:${string}` = StorageItemKey typechecks directly.
8. `resolveKey<G, StorageItemKey<G>>(key)` → `resolveKey(key)`. K
inferred from arg.
9. Area now narrowed. `KeyParts<'local:theme'>` yields `{driverArea:
'local'; driverKey: 'theme'}` — both halves. Previously the area was
still the full `'local' | 'session' | 'sync' | 'managed'` union
because `StorageItemKey<G>` distributed across all four areas.
## Per-method `<const K extends StorageItemKey>` at the interface
The correct place for key-literal narrowing is the *method*, not the
class. Applied to every single-key method on `WxtStorage`:
- `getItem<TValue, const K extends StorageItemKey = StorageItemKey>(key: K, ...)`
- `getMeta<const K>(key: K)`
- `setItem<const K>(key: K, value)`
- `setMeta<const K>(key: K, ...)`
- `removeItem<const K>(key: K, opts?)`
- `removeMeta<const K>(key: K, ...)`
- `watch<const K>(key: K, cb)`
Callers now see the specific key literal in hover. Internally, each
impl uses `<const K extends StorageItemKey>(key: K)` too, so
`resolveKey(key)` binds K and returns `ResolvedKey<K>` with narrowed
`driverArea: KeyParts<K>['driverArea']` and `driverKey:
KeyParts<K>['driverKey']`. driverKey is no longer just `string` in the
impl body — it narrows to the exact literal tail.
The `getItem<TValue>` variant keeps a `= StorageItemKey` default on K
so partial-explicit generics like `storage.getItem<string>('local:test',
{fallback: 'x'})` continue to work (TS doesn't infer K when TValue is
explicitly bound).
## StorageItemKey<G extends string = string> — the one G survives
Kept as an escape-hatch public alias. Default = `string` keeps every
existing bare `StorageItemKey` usage compatible; sophisticated users
who want to pin the name half can supply G explicitly
(`StorageItemKey<'theme' | 'lang'>` = the 8-element union).
## Verification
- `bun run check` clean (Oxlint, Publint, TypeScript strict).
- 286 tests pass.
- Smoke probe of every narrowing pathway checked separately.
## Files
- `packages/storage/src/types.ts` — KeyParts, WxtStorageItemLike,
GetItemsInputElement, GetItemsResult, GetMetasInputElement,
GetMetasResult all lose G.
- `packages/storage/src/index.ts` — WxtStorage, WxtStorageDriver,
WxtStorageItem, createStorage, createDriver, resolveKey, all
single-key methods restructured.
…d defaultValue
## setMeta helper: `unknown` → `Record<string, unknown> | null`
The stale JSDoc on the helper claimed `properties: unknown` was needed
"because upstream `setMeta<T>` on WxtStorage is caller-invented (Phase
D)." That comment was left over from an earlier design.
The current public signature already restricts to `Record<string,
unknown> | null` at the interface (`WxtStorage.setMeta`), and all three
callsites pass concrete objects:
- Line 478: dispatch from public `storage.setMeta` (already typed at
interface as `Record<string, unknown> | null`)
- Line 763: migration version bump — passes `{ v: targetVersion }`
- Line 835: `WxtStorageItem.setMeta` handler — bound to
`Partial<TMetadata> | null`
Widening to `unknown` inside the helper was over-permissive. Tightened
to match the public API. The runtime `getMetaValue(properties)`
coercion stays as belt-and-suspenders defense against non-object
metadata in storage (older writes, external mutations), which the
JSDoc now correctly explains.
## Restore `?? opts?.defaultValue` in getItem helper
The bulk `<G>` cleanup accidentally dropped
`?? opts?.defaultValue` from `getValueOrFallback`, breaking backward
compat for the deprecated `defaultValue` option. `GetItemOptions.defaultValue`
is still in the public type as `@deprecated`, and 4 tests exercise
it directly:
```ts
await storage.getItem(`${storageArea}:count`, { defaultValue: 0 });
// expected 0, was getting null
```
Restored `opts?.fallback ?? opts?.defaultValue` — fallback takes
precedence, defaultValue is the deprecated legacy path.
## No `driverKey: string` change
Confirmed the plain `string` typing on `driverKey` in internal helpers
is intentional and correct — see architectural note in code comments.
Threading `<const K extends string>` through helpers = Golden Rule
failure (K appears once at parameter, nowhere in return). A branded
`DriverKey` type would only pay for itself if helpers were exported;
they're all module-private.
## Verification
- `bun run check` clean.
- 286 tests pass (4 previously-failing `defaultValue` tests restored).
`KeyParts<K>` was pulling its weight for exactly two callsites — both
inside `resolveKey`, both as `KeyParts<K>['driverArea']` /
`['driverKey']` casts. Never accessed as a whole object, never re-
exported to a public consumer, never covered by a type test.
Plus the JSDoc lied. It claimed:
> `K = 'local:theme'` ⇒ `driverArea = 'local'`, `driverKey = 'theme'`,
> not a Cartesian product of areas × names
But the actual implementation used mapped-style with two INDEPENDENT
field-level conditionals:
```ts
{
readonly driverArea: K extends `${infer A extends StorageArea}:...` ? A : never;
readonly driverKey: K extends `${StorageArea}:${infer R}` ? R : never;
}
```
For `K = 'local:theme' | 'sync:userPrefs'`:
- `KeyParts<K>['driverArea']` = `'local' | 'sync'`
- `KeyParts<K>['driverKey']` = `'theme' | 'userPrefs'`
Fields computed independently ⇒ actually a Cartesian product,
including the impossible `{driverArea:'local', driverKey:'userPrefs'}`.
The distributive-conditional form that Phase F's fix commit
`4b7db583` moved AWAY from (to kill the `as unknown as` double cast)
was actually the one preserving correlation. The docstring didn't get
updated to match the refactor.
In practice K is always bound to a single literal via `<const K>` at
each call site, so the Cartesian product is a set of one and no bug
was ever visible. But the type WAS lying.
## Cleanup
- `packages/storage/src/types.ts`: delete the `KeyParts<K>` export,
its JSDoc, and the reference in the file-header docstring.
25 lines removed.
- `packages/storage/src/index.ts`: drop the `KeyParts` import; inline
the two template-literal splits directly in `resolveKey`. The type
parse now lives at the exactly-one point that reads it.
The two casts remain honest:
```ts
const driverArea = key.substring(0, i)
as K extends `${infer A extends StorageArea}:${string}` ? A : never;
const driverKey = key.substring(i + 1)
as K extends `${StorageArea}:${infer R}` ? R : never;
```
`infer A extends StorageArea` still constrains the area half to the
literal-union of `'local'|'session'|'sync'|'managed'` — the useful
narrowing survives the deletion, it just doesn't need a named type
to carry it.
## Verification
- `bun run check` clean (Oxlint, Publint, TypeScript strict).
- 286 tests pass.
- Zero `KeyParts` references remain in `packages/storage/src/`.
## Diff shape
- `packages/storage/src/index.ts` — 9 insertions / 10 deletions
(imports + resolveKey body)
- `packages/storage/src/types.ts` — 25 deletions (KeyParts + JSDoc)
15-lens advanced-type audit (typescript-advanced-type-audit skill) over
`packages/storage/src`. Baseline already strict: strict + noUncheckedIndexedAccess
+ exactOptionalPropertyTypes + noImplicitOverride + noImplicitReturns +
noFallthroughCasesInSwitch + noUncheckedSideEffectImports. Two actionable
findings; the rest of the surface is already at the precision ceiling.
## Finding 1 (L1/as-cast): self-contradicting `as StorageItemKey` — getItems
Line 374 read:
```ts
// Template literal narrows automatically: driverArea is StorageArea,
// driverResult.key is string, so the join is `${StorageArea}:${string}`
// = StorageItemKey.
const key = `${driverArea}:${driverResult.key}` as StorageItemKey;
```
The comment asserts the template literal narrows automatically, then
casts anyway. Both can't be true. `driverArea` is `StorageArea` (from
`Map<StorageArea, string[]>`), `driverResult.key` is `string`, so
`${driverArea}:${driverResult.key}` has type `${StorageArea}:${string}`,
which IS `StorageItemKey` (= `${StorageArea}:${string}` with the default
`G = string`). The cast was dead. Replaced with an annotation that lets
the compiler prove the assignment:
```ts
const key: StorageItemKey = `${driverArea}:${driverResult.key}`;
```
If the join type ever drifts from `StorageItemKey`, this now errors
instead of silently swallowing it under `as`.
## Finding 2 (L15/non-null): `map[key.driverArea]!.push()` — getMetas
Line 421-422 read:
```ts
map[key.driverArea] ??= [];
map[key.driverArea]!.push(key);
```
`noUncheckedIndexedAccess` types the index read as `T | undefined`, so
the `!` papered over the re-read. `??=` already returns the resolved
(never-undefined) value — capture it:
```ts
const bucket = (map[key.driverArea] ??= []);
bucket.push(key);
```
Zero non-null assertions now remain in `packages/storage/src`.
## Lenses that yielded NOTHING (audited, honest zero)
- L1 `any`: only `(oldValue: any)` in the migration chain — intentional
and documented (heterogeneous chain, param contravariance). Kept.
- L9 discriminated unions: options interfaces are independent flags, not
always-together sub-groups. Not candidates.
- L10 `satisfies`: the `Record<...>` literals are uniform-value maps;
`satisfies` gains nothing over the annotation.
- L12 ReturnType dup: the `as ReturnType<typeof defineMigrations>` is the
canonical overload-implementation cast, not a duplicate interface.
- L13 boundaries: `watch()`'s `changes[key] as {newValue?: unknown; ...}`
TIGHTENS `StorageChange`'s `any` fields to `unknown` — already a win.
- L14 Record-vs-Map: every `Record<string, unknown>` with `delete` is a
storage-serialized plain object; a Map would break serialization.
- L8 template-literal unions: `excludeKeys` holds area-stripped bare
names; `string` is correct (no prefix to narrow).
## Verification
- `bun run check` clean (Oxlint, Publint, TypeScript strict).
- 286 tests pass.
- `rg -n '!\.|!\[|!\)' packages/storage/src --glob '!__tests__/**'` = 0.
… guards
Three-part test-suite cleanup. Runtime tests: 286 → 161 (removed 125
zero-coverage-delta executions), then +6 for the new guards/routing =
167 runtime + 149 type assertions now counted = 316 total. Coverage:
96.99→97.89 stmt, 92.99→94.9 branch.
## 1. Collapse the 4× area parameterization (zero coverage loss)
`describe.each(['local','sync','managed','session'])` re-ran the entire
~35-behavior Storage Utils block once per area. But the area is a plain
`browser.storage[area]` lookup and fakeBrowser models all four areas
identically — runs 2/3/4 re-proved the same code paths with a different
lookup string and hit ZERO new branches.
Pinned the block to `local` (one representative run) and added a single
`describe('storage area routing')` with an `it.each` over all four areas
that proves get/set thread to the correct `browser.storage[area]` — the
only thing the 4× runs actually covered that a single-area run doesn't.
Empirical proof it was pure redundancy: coverage was byte-identical
(96.99/92.99/96.9/96.88) before and after dropping the 125 executions.
## 2. Make the 46 `expectTypeOf` assertions first-class test cases
`vitest.config.ts` had no `typecheck` block, so every `expectTypeOf` was
a runtime no-op under `vitest run` — validated only by `tsc` during
`bun run check`, invisible to the runner. Enabled:
```ts
typecheck: { enabled: true, include: ['**/*.test.ts'] }
```
`vitest run` now reports "Type Errors: no errors" and counts the type
assertions. The type tests were always there and always enforced by CI's
check step; this just surfaces them as visible cases.
## 3. Cover the three getStorageArea environment guards
Lines 886/894/901 (missing `browser.runtime`, missing `browser.storage`,
undefined area) were uncovered. Added `describe('getStorageArea
environment guards')` — each test save/null/restores the module `browser`
binding so a driver op walks into the guard, asserting the specific throw
message. Branch coverage 92.99 → 94.9.
## Remaining uncovered (deliberate)
- `1410-1503`: `defineMigrations` type-only overload signatures — never
execute by design.
- `81`: unreachable defensive throw (`substring` never returns null).
- `729`/`778`: timing-dependent eager migration/init async-failure paths;
a deterministic test would be brittle. Left uncovered per the
don't-manufacture-coverage principle.
## Verification
- `bun run check` clean (Oxlint, Publint, TypeScript strict).
- `bun run test run`: 316 passed, Type Errors: no errors.
- Coverage: 97.89% stmt / 94.9% branch / 96.9% func / 97.81% line.
- Bump @wxt-dev/storage to 2.0.0 (semver-major, seven public-API breaks) - Pin @standard-schema/utils ~0.3.0 to guard against pre-1.0 minor breaks - Prepend v2.0.0 entry to packages/storage/CHANGELOG.md documenting every break, enhancement, refactor, test, and doc change - Add '@wxt-dev/storage v1.x → v2.0' section to docs/guide/resources/upgrading.md with before/after snippets for all seven tier-1 breaks (five caller-invented generic removals, restoreSnapshot any→Record, migrations object→tuple, getItems result narrowing) - Expand 'Schema Validation' section in docs/storage.md with a validator cookbook: Zod, Valibot, ArkType, Effect Schema all shown defining the same theme item (closes DIR-01 from branch audit) Covers findings DEP-01, DEP-02, DOCS-01, DOCS-02, DIR-01 from advisor-plans/branch-audit-9e69b39b.md. Build gate (bun run buildc all) verified clean, closing DEBT-01.
TWO IDE / hover improvements built on the existing strict-type overload stack:
1. Prettify<T> shell forces eager mapped-type evaluation on hover
---------------------------------------------------------------
Before: hovering `bookmarks.version` displayed
'(property) WxtStorageItem<...>.version: TVersion'
-- referencing the interface's declared type-parameter rather than the
instantiated value. The type CHECKER always saw the resolved literal, so
assignability was correct; only the DISPLAY was cluttered.
After: every `defineItem` overload returns `Prettify<WxtStorageItem<...>>`
where `Prettify<T> = { [K in keyof T]: T[K] } & {}`. TS eagerly maps each
member key, resolving every type-parameter reference. Hovering
`bookmarks.version` now shows the literal `3`. Same for `.debug`,
`.key`, `.fallback`, and every method's resolved return type.
Pattern references: type-fest Simplify, Matt Pocock Prettify, TS #47980.
2. `<const TMigrations>` capture on defineItem preserves chain-checked signatures
----------------------------------------------------------------------------
Before: hovering `migrations` in the options bag displayed
'migrations?: readonly [MigrationFn, MigrationFn]'
-- the length lock via `MigrationTuple<TVersion>` worked, but each slot's
fn signature widened to the bare `MigrationFn` alias
`(oldValue: any) => unknown | Promise<unknown>`. The narrow chain-checked
fns produced by `defineMigrations<T>()` (`(v: unknown) => T` /
`(v: T) => T`) were structurally assignable to `MigrationFn` and got
collapsed on display.
After: every non-bare `defineItem` overload declares
const TMigrations extends
MigrationTuple<TVersion> = MigrationTuple<TVersion>,
and the options intersection is
Omit<WxtStorageItemOptions<...>, 'migrations'> & {
migrations?: TMigrations;
...
}
With `<const TMigrations>` capture and the base interface's `migrations`
field omitted, the narrow tuple from `defineMigrations` (and any raw
literal tuple) flows through unchanged. Hover shows each fn's actual
per-slot signature. The `MigrationTuple<TVersion>` constraint retains the
length lock so `version: 3` still requires exactly 2 slots.
Related fix: MigrationTuple<0> guard
------------------------------------
`storage.defineItem('key', { version: 0 })` used to hit an infinite
type-instantiation loop (Subtract<0,1> → never; BuildTuple<never> recurses
forever). The guard `V extends 0 | 1 ? readonly [] : ...` in
`MigrationTuple` short-circuits both invalid `version: 0` and valid
`version: 1` (which needs no migrations) to `readonly []`.
Tests:
* hover-eager-resolve.test-d.ts \u2014 pins accessor type resolution to
literals (`3`, `true`, `'sync:bookmarks'`, narrow-readonly fallback shape,
resolved schema type, resolved getValue return).
* migrations-narrow.test-d.ts \u2014 pins defineMigrations chain-checked shape,
length-lock enforcement (assignability test, not directive-based since
vitest-typecheck harness struggles with `@ts-expect-error` on nested
diagnostics), and version:0 type-termination.
* vitest.config.ts \u2014 typecheck.include now covers .test-d.ts files so all
3 pre-existing type tests + 2 new ones run through the vitest harness.
7 test files, 352 tests, 0 type errors, 0 tsc errors.
Add a per-area brand so 'local' / 'sync' / 'managed' / 'session' drivers
and items are distinguishable at compile time.
1. WxtStorageDriver<TArea extends StorageArea>
------------------------------------------
Parametric brand on the internal driver abstraction. `createDriver`
captures the literal via `<const TArea>`, so:
createDriver('local') -> WxtStorageDriver<'local'>
createDriver('managed') -> WxtStorageDriver<'managed'>
TArea sits in a read-only output position (`readonly area: TArea`),
so it's covariant \u2014 narrow drivers assign to wide slots.
Runtime: each driver returns `{ area: storageArea, ... }`. No extra
state \u2014 the closure already held `storageArea`; we just expose it.
2. WxtStorageItem.area \u2014 template-literal-derived
----------------------------------------------
New public accessor:
readonly area: TKey extends `${infer A extends StorageArea}:${string}`
? A
: StorageArea;
For `storage.defineItem('local:count', ...)`, `item.area` is the
literal `'local'`. For a wide key, it widens back to `StorageArea`.
Runtime: pulled from the underlying driver via `driver.area` \u2014 no
duplicated state, and `item.area === driver.area` holds for every
item routed through the same driver.
Not linked at the type level: `.area` on the item derives from `TKey`,
not threaded through `TArea` on the driver. That's intentional \u2014 the
singleton dispatches on the key's string prefix, so the per-item narrow
area doesn't need to flow through the driver's phantom.
WxtStorageDriver stays internal (unexported). The user-visible surface
is `.area` on every item.
Follow-up potential: specialize per-area driver interfaces \u2014 e.g. omit
`setItem` / `clear` / `restoreSnapshot` from `WxtStorageDriver<'managed'>`
since managed is read-only in the WebExtensions API. That would move the
'managed is read-only' invariant from runtime docs into the type system.
Tests:
* area-brand.test-d.ts \u2014 pins .area narrowing for each of the four
literal prefixes plus the widened fallback path, and demonstrates
exhaustive runtime discrimination.
8 test files, 358 tests, 0 type errors, 0 tsc errors.
Split WxtStorageDriver into Base | Mutable | Readonly via a distributing
conditional type, encoding the WebExtensions 'managed storage is read-only'
constraint in the type system rather than runtime docs alone.
Type-level changes
------------------
WxtStorageDriver<T extends StorageArea = StorageArea>
= T extends 'managed'
? ReadonlyStorageDriver<T>
: MutableStorageDriver<T>
- BaseStorageDriver<TArea> — reads + watch/unwatch + area brand
- MutableStorageDriver<TArea> — extends Base with setItem / setItems /
removeItem / removeItems / clear /
restoreSnapshot
- ReadonlyStorageDriver<TArea> — alias for Base, omits all write methods
- WxtStorageDriver<TArea> — distributing conditional that picks
Readonly<'managed'> or Mutable<other>
WxtStorageDriver<'managed'>.setItem is now a compile-time error.
Distribution across WxtStorageDriver<StorageArea> (the union default)
yields Readonly<'managed'> | Mutable<'local'|'sync'|'session'>, so
internal callers with a wide area cannot accidentally call mutating
methods without first narrowing.
Runtime surface
---------------
assertMutable(driver) — assertion function inside createStorage that:
- narrows WxtStorageDriver<StorageArea> -> MutableStorageDriver
- throws with WXT-branded "Cannot write to browser.storage.managed"
error instead of the raw browser rejection
Inserted at every internal write path:
processReadValue (reset branch), setItem helper, setMeta helper,
removeItem helper, removeMeta helper, setItems/setMetas/removeItems
batch dispatch loops, clear, restoreSnapshot, defineItem's eager-init
write and migrations write.
Return-site cast
----------------
createDriver returns the full mutable literal (the impl always has all
methods — the browser is what rejects managed writes at runtime). A
single cast at the return site erases
excess-property checking for the Readonly branch. Cast is documented
inline; no satisfies workaround used.
Test changes
------------
- Split the routing it.each to exclude 'managed' from the set/get loop
- Added 'routes reads to browser.storage.managed' (seeds via fakeBrowser
internals; verifies reads still route correctly)
- Added 'rejects writes to browser.storage.managed with a clear error'
(setItem, removeItem, clear, restoreSnapshot all throw the WXT error)
- Updated area-brand.test-d.ts preamble to document the 3-mechanism
hierarchy (brand + item.area + read-only split)
8 test files, 361 tests, 0 type errors, 0 tsc errors
….read, migration pre-write validation
index.ts
- Add isRecord(v): v is Record<string,unknown> type predicate
Removes the only remaining 'as' cast from getMetaValue — the
predicate body is equivalent to the inline ternary condition but
provably correct and reusable across the codebase.
- Validate migrated value against schema BEFORE writing to disk
A buggy migration fn previously wrote invalid data with the bumped
version, making schema validation on getValue() fail permanently
(version advanced, migration won't re-run). Now throws MigrationError
before driver.setItems — version stays un-bumped, next load retries.
Upholds aklinker's 'writes always throw on invalid' invariant.
types.ts
- WxtStorageItemSerializer.read: property -> method shorthand (bivariant)
read?: (raw: TRaw) => TValue -> read?(raw: unknown): TValue
Method shorthand is checked bivariantly under strictFunctionTypes so
users can declare a narrower param type without a cast:
read(raw: MyWireType): T { ... } // ok — no 'as' needed
Trust boundary enforced by the pipeline (passes unknown from storage).
io-ts Type<A,O,I> pattern: I = unknown at the boundary, impl narrows.
tests
- Fix two serializer.read tests to use Array.isArray type guard
instead of new Set(raw) — raw is now unknown not TRaw, guard needed
Critical fixes from cross-review:
- migrate() now routes migrated output through processWriteValue so
schema + serializer.write apply before persisting. Previously wrote
the runtime form directly, so serializer-backed items would store
wire-form garbage and next read via serializer.read would fail.
SchemaError is wrapped as MigrationError so version stays un-bumped
and next load retries.
- getItems([item]) now routes item slots through the item's own read
pipeline via new _processRead(raw) internal method. Types promised
{value: TValue} but runtime returned raw storage — schema, serializer,
and onValidationError were all bypassed. Batching is preserved
(single driver.getItems round-trip per area) and each item's pipeline
runs in-memory on the pre-fetched raw.
Should-fix from cross-review:
- CHANGELOG onValidationError variants corrected from 'log'|'ignore'|
(err,ctx)=>T to 'throw'|'fallback'|'reset'|(issues,raw)=>T.
- CHANGELOG defineMigrations shape corrected from .add(fn) builder to
variadic (fn1, fn2, ...) with 6 typed overloads.
- Added test:types npm script.
- Test count updated in CHANGELOG (316 -> 367 runtime tests).
Other cleanups:
- getMetaValue's rawMetaValue cast replaced with isRecord() predicate
for consistency (line 708).
- OnValidationError<TValue> | 'throw' redundancy removed (TValue union
already includes 'throw').
- Comment noise swept: removed multi-paragraph rationales that just
restate code intent, redundant JSDoc that repeats the function name,
and inline // notes narrating obvious control flow. index.ts dropped
by 105 lines, types.ts by 67 lines, without losing any non-obvious
invariant or contract documentation. Kept: public API JSDoc, cast
justifications (contravariance, DeepReadonly boundary, impl-sig),
and warnings about non-obvious behavior.
Tests added (367 total, +6):
- Migration output failing schema throws MigrationError, version
stays un-bumped.
- Migrated value flows through serializer.write.
- Schema-transformed migration value passed to onMigrationComplete.
Docs updated:
- docs/storage.md 'Running Migrations' section now describes the
write-pipeline application on migration output (schema throws
MigrationError, serializer.write applied).
## Scope cleanup (user request)
- Remove .test-d.ts files (kept local, not part of PR surface)
- Remove advisor-plans/ (planning artifacts, not shipping)
- Revert docs/storage.md, docs/guide/resources/upgrading.md — docs
updates left to maintainer discretion
- Revert CHANGELOG.md — release notes belong on the release commit
- Revert vitest.config.ts (typecheck no longer needed without .test-d.ts)
- Remove test:types script (no more type-only test files)
- Remove zod from devDependencies and from test file
* Rewrote 3 tests that used zod to use the built-in defineSchema<T>
adapter instead — same behavior, one fewer dev dep
## Fix reviewer's real bugs
### Batch race with onValidationError: 'reset' (Critical)
getItems previously batched raw reads BEFORE awaiting per-item
migrations. If a migration was still running, batch captured pre-
migration raw, awaited migrationsDone, then processed stale raw
through the pipeline. With onValidationError: 'reset', schema failure
on stale raw could delete the newly-migrated value.
Fix: getItems now awaits every item slot's readiness (migrations +
eager init) BEFORE the raw batch fetch. State captured post-migration,
post-init. Belt-and-suspenders: batch's processRead uses
allowReset: false so a stale raw slipping through cannot destroy data.
### _processRead doesn't run init (Should-Fix)
Init items previously received pre-fetched raw through processReadValue,
bypassing the init logic (init runs writes on empty storage). Batch's
processRead now delegates to getOrInitValue for init items, matching
getValue()'s semantics.
### _processRead leaked into public .d.ts (Should-Fix)
Adding _processRead as a required member of WxtStorageItem interface
made it a public API surface (comment-based @internal is not enforced
by tsc). Moved to a module-level WeakMap<item, hooks> keyed by item
identity. Zero public surface, zero leaked types.
## Regression tests
- getItems routes item slots through schema/serializer (was untyped)
- getItems waits for migrations before batch read (no stale data)
- getItems with onValidationError: 'reset' does not destroy data on
stale batch reads
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
the
pkg/storage
index.ts drops from 1843 to 1551 LOC. Four new files, each named for
one concern:
src/driver.ts driver types + assertMutable
src/migrations.ts defineMigrations, DefineMigrations, MigrationError
src/pipeline.ts runReadPipeline, runWritePipeline, adapter types,
defineSchema, PipelineValidateResult
src/pipeline-adapters.ts processReadValue, processWriteValue as pure
module-level functions taking driver/key/opts
Public API unchanged; every export from index.ts is preserved via
re-export from the new modules where relocated. 171/171 tests pass,
TypeScript clean under all strict flags.
Type-safety wins from the refactor:
- PipelineValidateResult<TOut> is now StandardSchemaV1.Result<TOut>,
the same discriminated union Standard Schema itself uses. The old
inline pattern in processReadValue / processWriteValue required TS
to work around a non-discriminated shape; the extracted pipeline
narrows cleanly on `if (result.issues)`, so `result.value` types
as TOut with no cast.
- The adapter shape (hasSchema, passthrough, preValidate, validate,
fallback, onValidationError, resetUnderLock) is a single place to
reason about the read pipeline. Encoder-at-rest and other future
serializers plug in via the adapter without touching pipeline code.
- Zero `as any` / `as never` / `as unknown as` escapes in the new
files. Every generic in the pipeline satisfies the Golden Rule
(each type parameter appears \u2265 2 times in the signature).
No behavioural change: the four extracted functions retain their
original semantics; only their location changes.
Two type-safety fixes surfaced during a hover audit of the schema
overloads:
- Default TMetadata on every defineItem overload was Record<string,unknown> = {}.
When a caller does not configure metadata (the current state — no meta
option yet), the empty-object default made NullablePartial<TMetadata>
collapse to a shape with no accessible keys, so item.setMeta(...) offered
nothing useful in IntelliSense. Changed the default to
Record<string, unknown> so untyped meta is an open string-keyed record.
- getValueOrFallback and defineItem.getFallback returned the raw fallback
reference. Multiple getValue() calls on an empty row returned the same
array/object identity; a caller's mutation leaked across reads.
structuredClone materializes a fresh copy at each fallback return.
Both are type-only or type-adjacent runtime fixes. 172/172 tests pass.
Contributor
Author
|
@aklinker1 landed the value-side per #1173 — schema at root, nested serializer: { read, write }, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1173.
Type surface