From d4e668b23433e68c1d182d8ccda947434c26c29f Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 24 May 2026 22:44:52 +0200 Subject: [PATCH] refactor(soma): revert context.set try/catch; mock pickerShellContext explicitly in picker tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Kim audit Round 3 §2: the broad try/catch around `ctx.set` in `src/uix/soma/provider/context.ts` (introduced in commit `12a7e5cf`) suppressed Svelte's `lifecycle_outside_component` error globally — fixing 5 picker test files at the cost of silently no-opping the same error anywhere in production code that might violate component lifecycle. Aggressive in scope for what was actually a test-harness omission. This commit: - Reverts `context.ts` to the plain `ctx.set(value)` pattern. Other error semantics (the typed `get()` throw + `getOr` fallback) stay intact. Adds a docblock noting that providers calling `ctx.set(...)` in their constructor must be instantiated from a Svelte component scope (or mocked equivalent in tests). - Adds `vi.spyOn(pickerShellContext, 'set').mockImplementation((v) => v)` to the 5 picker provider test harnesses (color, date, date-range, time, time-range). Mirrors the pattern those tests already used for their own XProvider.ctx — explicit, scoped to tests, visible. Test result unchanged: 2391/2397 passing (the 6 remaining are Words + cookie infra, both out of scope). No regressions in the picker tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../color-picker-provider.svelte.test.ts | 5 +++ .../date-picker-provider.svelte.test.ts | 5 +++ .../date-range-picker-provider.svelte.test.ts | 5 +++ .../time-picker-provider.svelte.test.ts | 5 +++ .../time-range-picker-provider.svelte.test.ts | 5 +++ src/uix/soma/provider/context.ts | 34 +++++++------------ 6 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/uix/soma/components/color-picker/color-picker-provider.svelte.test.ts b/src/uix/soma/components/color-picker/color-picker-provider.svelte.test.ts index 7528c85f9..8fa9fc4c2 100644 --- a/src/uix/soma/components/color-picker/color-picker-provider.svelte.test.ts +++ b/src/uix/soma/components/color-picker/color-picker-provider.svelte.test.ts @@ -32,6 +32,7 @@ import { } from './color-picker-provider.svelte'; import type { ColorChannel } from './types'; import type { ColorOnInvalid, ColorValidator } from '../color-field/types'; +import { pickerShellContext } from '../picker-shell'; function withEffectRoot(fn: () => T): { result: T; cleanup: () => void } { let result!: T; @@ -60,6 +61,10 @@ function installSomaHarness() { vi.spyOn(FieldProvider, 'get').mockReturnValue(undefined); vi.spyOn(ColorPickerProvider.ctx, 'set').mockImplementation((value) => value); vi.spyOn(ColorPickerAreaProvider.ctx, 'set').mockImplementation((value) => value); + // PickerShell handle is registered by the provider constructor via + // Svelte's setContext — mock it so direct `new` outside a component + // doesn't hit the lifecycle_outside_component error. + vi.spyOn(pickerShellContext, 'set').mockImplementation((value) => value); return { dom }; } diff --git a/src/uix/soma/components/date-picker/date-picker-provider.svelte.test.ts b/src/uix/soma/components/date-picker/date-picker-provider.svelte.test.ts index 7e87495ee..016e1ef60 100644 --- a/src/uix/soma/components/date-picker/date-picker-provider.svelte.test.ts +++ b/src/uix/soma/components/date-picker/date-picker-provider.svelte.test.ts @@ -25,6 +25,7 @@ import { type DatePickerKind, type DatePickerMode } from './date-picker-provider.svelte'; +import { pickerShellContext } from '../picker-shell'; function withEffectRoot(fn: () => T): { result: T; cleanup: () => void } { let result!: T; @@ -52,6 +53,10 @@ function installSomaHarness() { vi.spyOn(Soma, 'require').mockReturnValue(soma); vi.spyOn(DatePickerProvider.ctx, 'set').mockImplementation((value) => value); + // PickerShell handle is registered by the provider constructor via + // Svelte's setContext — mock it so direct `new` outside a component + // doesn't hit the lifecycle_outside_component error. + vi.spyOn(pickerShellContext, 'set').mockImplementation((value) => value); return { dom }; } diff --git a/src/uix/soma/components/date-range-picker/date-range-picker-provider.svelte.test.ts b/src/uix/soma/components/date-range-picker/date-range-picker-provider.svelte.test.ts index 459cd7c62..cf9a8f8b7 100644 --- a/src/uix/soma/components/date-range-picker/date-range-picker-provider.svelte.test.ts +++ b/src/uix/soma/components/date-range-picker/date-range-picker-provider.svelte.test.ts @@ -22,6 +22,7 @@ import { createSomaRuntime, type SomaRuntimeSources } from '$soma/runtime.svelte import type { Direction, OnChangeFn } from '../../types'; import { DateRangePickerProvider } from './date-range-picker-provider.svelte'; +import { pickerShellContext } from '../picker-shell'; function withEffectRoot(fn: () => T): { result: T; cleanup: () => void } { let result!: T; @@ -49,6 +50,10 @@ function installSomaHarness() { vi.spyOn(Soma, 'require').mockReturnValue(soma); vi.spyOn(DateRangePickerProvider.ctx, 'set').mockImplementation((value) => value); + // PickerShell handle is registered by the provider constructor via + // Svelte's setContext — mock it so direct `new` outside a component + // doesn't hit the lifecycle_outside_component error. + vi.spyOn(pickerShellContext, 'set').mockImplementation((value) => value); return { dom }; } diff --git a/src/uix/soma/components/time-picker/time-picker-provider.svelte.test.ts b/src/uix/soma/components/time-picker/time-picker-provider.svelte.test.ts index 0105cdf06..7f7c12737 100644 --- a/src/uix/soma/components/time-picker/time-picker-provider.svelte.test.ts +++ b/src/uix/soma/components/time-picker/time-picker-provider.svelte.test.ts @@ -19,6 +19,7 @@ import { createSomaRuntime, type SomaRuntimeSources } from '$soma/runtime.svelte import type { Direction, OnChangeFn } from '../../types'; import { TimePickerProvider } from './time-picker-provider.svelte'; +import { pickerShellContext } from '../picker-shell'; function withEffectRoot(fn: () => T): { result: T; cleanup: () => void } { let result!: T; @@ -49,6 +50,10 @@ function installSomaHarness(hourCycle?: HourCycle) { vi.spyOn(Soma, 'require').mockReturnValue(soma); vi.spyOn(TimePickerProvider.ctx, 'set').mockImplementation((value) => value); + // PickerShell handle is registered by the provider constructor via + // Svelte's setContext — mock it so direct `new` outside a component + // doesn't hit the lifecycle_outside_component error. + vi.spyOn(pickerShellContext, 'set').mockImplementation((value) => value); return { dom }; } diff --git a/src/uix/soma/components/time-range-picker/time-range-picker-provider.svelte.test.ts b/src/uix/soma/components/time-range-picker/time-range-picker-provider.svelte.test.ts index b70b996a2..f75cea0f7 100644 --- a/src/uix/soma/components/time-range-picker/time-range-picker-provider.svelte.test.ts +++ b/src/uix/soma/components/time-range-picker/time-range-picker-provider.svelte.test.ts @@ -20,6 +20,7 @@ import { createSomaRuntime, type SomaRuntimeSources } from '$soma/runtime.svelte import type { Direction, OnChangeFn } from '../../types'; import { TimeRangePickerProvider } from './time-range-picker-provider.svelte'; +import { pickerShellContext } from '../picker-shell'; function withEffectRoot(fn: () => T): { result: T; cleanup: () => void } { let result!: T; @@ -50,6 +51,10 @@ function installSomaHarness(hourCycle?: HourCycle) { vi.spyOn(Soma, 'require').mockReturnValue(soma); vi.spyOn(TimeRangePickerProvider.ctx, 'set').mockImplementation((value) => value); + // PickerShell handle is registered by the provider constructor via + // Svelte's setContext — mock it so direct `new` outside a component + // doesn't hit the lifecycle_outside_component error. + vi.spyOn(pickerShellContext, 'set').mockImplementation((value) => value); return { dom }; } diff --git a/src/uix/soma/provider/context.ts b/src/uix/soma/provider/context.ts index b55180078..1a0b8a794 100644 --- a/src/uix/soma/provider/context.ts +++ b/src/uix/soma/provider/context.ts @@ -2,33 +2,23 @@ import { Context } from 'runed'; import { SomaProviderContextNotFoundError } from '../errors'; /** - * Creates a typed context for a soma component. + * Creates a typed context for a soma component. Thin wrapper over + * `runed`'s `Context` adding a typed `get()` that throws a domain + * error when the descendant reads context outside its provider scope, + * and a permissive `getOr(fallback)`. * - * `set` swallows Svelte's `lifecycle_outside_component` error so unit - * tests can instantiate providers directly (`new XProvider(opts)`) - * without a component tree to host the context. In that mode there's - * nothing reading the context anyway — descendants don't exist — so - * silently no-opping is the right semantic. Inside a real component - * the call proceeds normally and registers the value. + * Note on tests: providers that call `ctx.set(...)` in their constructor + * — e.g. picker providers registering `pickerShellContext` — must be + * instantiated from inside a Svelte component scope (or its mocked + * equivalent). Unit tests that do `new XProvider(opts)` directly need + * to `vi.spyOn(XCtx, 'set').mockImplementation((v) => v)` on every + * context the provider touches, so Svelte's `setContext` no-op outside + * components doesn't reach the call site. */ export function context(name: string) { const ctx = new Context(name); return { - set: (value: T) => { - try { - ctx.set(value); - } catch (err) { - const message = (err as { code?: string; message?: string })?.code ?? ''; - const text = (err as Error)?.message ?? ''; - if ( - message !== 'lifecycle_outside_component' && - !text.includes('lifecycle_outside_component') && - !text.includes('setContext') - ) { - throw err; - } - } - }, + set: (value: T) => ctx.set(value), get: (): T => { const value = ctx.getOr(null as unknown as T); if (value === null || value === undefined) {