From d36c2627ad7d0e8a9a2c2bc07a1d01b5a4889d47 Mon Sep 17 00:00:00 2001 From: Paul Lange <581407+codeart1st@users.noreply.github.com> Date: Mon, 18 Aug 2025 07:31:07 +0000 Subject: [PATCH 1/4] fix: make has-trap more resilient In unit tests without .peek() or .get(), we encountered many exceptions due to the behavior of vitest and chai when iterating over objects for equality checks and error printing. Making this more resilient without throwing exceptions seems like a useful improvement for me. --- src/ObservableObject.ts | 20 +++++++++++++++++++- tests/tests.test.ts | 10 ++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index f09bdb7a..7038010f 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -1,6 +1,7 @@ import { checkPlain } from './checkPlain'; import { beginBatch, createPreviousHandler, endBatch, isArraySubset, notify } from './batching'; import { createObservable } from './createObservable'; +import { observable } from './observable'; import { equals, extractFunction, @@ -412,6 +413,10 @@ const proxyHandler: ProxyHandler = { } } + if (p === 'constructor') { + return observable; + } + let value = peekInternal(node, /*activateRecursive*/ p === 'get' || p === 'peek'); // Trying to get an iterator if the raw value is a primitive should return undefined. @@ -639,7 +644,20 @@ const proxyHandler: ProxyHandler = { }, has(node: NodeInfo, prop: string) { const value = getNodeValue(node); - return Reflect.has(value, prop); + + // Short-circuit for the inputs that make Reflect.has error. + if (value === undefined || value === null) { + return false; + } + + // Functions behave like objects here, so let them flow through. + if (typeof value === 'object' || typeof value === 'function') { + return Reflect.has(value, prop); + } + + // For primitives (number, string, boolean, bigint, symbol) report “no key”. + // That keeps inspection code happy without pretending properties exist. + return false; }, apply(target, thisArg, argArray) { // If it's a function call it as a function diff --git a/tests/tests.test.ts b/tests/tests.test.ts index 944e22fa..eff57589 100644 --- a/tests/tests.test.ts +++ b/tests/tests.test.ts @@ -3881,4 +3881,14 @@ describe('Misc', () => { expect(descriptProxy).toEqual(undefined); expect(descriptValue).toEqual(undefined); }); + test('Observable is compatible with vitest equality checks and error printing', () => { + const obs = observable({ foo: 'bar' }); + const obj = obs.foo + + // https://github.com/vitest-dev/vitest/blob/v3.2.4/packages/expect/src/jest-utils.ts#L42-L49 + expect(!!obj && typeof obj === 'object' && 'asymmetricMatch' in obj).toBe(false) + + // https://github.com/vitest-dev/vitest/blob/v3.2.4/packages/pretty-format/src/index.ts#L288 + expect(`${obj.constructor.name}`).toBe('observable') + }); }); From 9021bbfd3905bc6fa51b455f0c290bce7222e2c9 Mon Sep 17 00:00:00 2001 From: Paul Lange <581407+codeart1st@users.noreply.github.com> Date: Mon, 1 Dec 2025 05:23:00 +0000 Subject: [PATCH 2/4] fix: remove circular import and use dummy constructor instead of real one --- src/ObservableObject.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index 7038010f..fc886c9f 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -1,7 +1,6 @@ import { checkPlain } from './checkPlain'; import { beginBatch, createPreviousHandler, endBatch, isArraySubset, notify } from './batching'; import { createObservable } from './createObservable'; -import { observable } from './observable'; import { equals, extractFunction, @@ -414,7 +413,7 @@ const proxyHandler: ProxyHandler = { } if (p === 'constructor') { - return observable; + return function observable() {}; } let value = peekInternal(node, /*activateRecursive*/ p === 'get' || p === 'peek'); From e734f6e7a742c021359148891cea8f93d8e507da Mon Sep 17 00:00:00 2001 From: Jay Meistrich Date: Tue, 2 Dec 2025 10:43:22 +0100 Subject: [PATCH 3/4] tweak constructor check to return any existing value if available first --- src/ObservableObject.ts | 3 ++- tests/tests.test.ts | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index fc886c9f..24911aed 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -413,7 +413,8 @@ const proxyHandler: ProxyHandler = { } if (p === 'constructor') { - return function observable() {}; + const ctor = peekInternal(node)?.constructor; + return typeof ctor === 'function' ? ctor : Object; } let value = peekInternal(node, /*activateRecursive*/ p === 'get' || p === 'peek'); diff --git a/tests/tests.test.ts b/tests/tests.test.ts index eff57589..f8fda2f9 100644 --- a/tests/tests.test.ts +++ b/tests/tests.test.ts @@ -3883,12 +3883,14 @@ describe('Misc', () => { }); test('Observable is compatible with vitest equality checks and error printing', () => { const obs = observable({ foo: 'bar' }); - const obj = obs.foo + const obj = obs.foo; + const ctor = obj.constructor; // https://github.com/vitest-dev/vitest/blob/v3.2.4/packages/expect/src/jest-utils.ts#L42-L49 - expect(!!obj && typeof obj === 'object' && 'asymmetricMatch' in obj).toBe(false) + expect(!!obj && typeof obj === 'object' && 'asymmetricMatch' in obj).toBe(false); // https://github.com/vitest-dev/vitest/blob/v3.2.4/packages/pretty-format/src/index.ts#L288 - expect(`${obj.constructor.name}`).toBe('observable') + expect(typeof ctor).toBe('function'); + expect(obj.constructor).toBe(ctor); }); }); From 06a6fea0f4514d1ee37ab3c2e71ba8c80f4f4e18 Mon Sep 17 00:00:00 2001 From: Paul Lange <581407+codeart1st@users.noreply.github.com> Date: Tue, 2 Dec 2025 16:37:15 +0000 Subject: [PATCH 4/4] style: fix formatting --- src/observableTypes.ts | 3 ++- src/sync-plugins/crud.ts | 6 ++++-- src/sync-plugins/fetch.ts | 6 ++++-- src/sync-plugins/firebase.ts | 3 ++- src/sync-plugins/keel.ts | 28 +++++++++++++++++----------- src/sync-plugins/supabase.ts | 13 ++++++------- src/sync-plugins/tanstack-query.ts | 12 ++++++++---- src/sync/syncTypes.ts | 9 ++++----- 8 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/observableTypes.ts b/src/observableTypes.ts index 76e52501..99c8fc48 100644 --- a/src/observableTypes.ts +++ b/src/observableTypes.ts @@ -43,7 +43,8 @@ export type RemoveObservables = : T; interface ObservableArray - extends ObservablePrimitive, + extends + ObservablePrimitive, Pick>, ArrayOverrideFnNames>, Omit>, ArrayOverrideFnNames> {} diff --git a/src/sync-plugins/crud.ts b/src/sync-plugins/crud.ts index 38cd8a53..dee705bd 100644 --- a/src/sync-plugins/crud.ts +++ b/src/sync-plugins/crud.ts @@ -69,8 +69,10 @@ export interface CrudErrorParams extends Omit { export type CrudOnErrorFn = (error: Error, params: CrudErrorParams) => void; -export interface SyncedCrudPropsBase - extends Omit, 'get' | 'set' | 'initial' | 'subscribe' | 'waitForSet' | 'onError'> { +export interface SyncedCrudPropsBase extends Omit< + SyncedOptions, + 'get' | 'set' | 'initial' | 'subscribe' | 'waitForSet' | 'onError' +> { create?(input: TRemote, params: SyncedSetParams): Promise | null | undefined | void>; update?( input: Partial, diff --git a/src/sync-plugins/fetch.ts b/src/sync-plugins/fetch.ts index 0205ae6d..2456960f 100644 --- a/src/sync-plugins/fetch.ts +++ b/src/sync-plugins/fetch.ts @@ -8,8 +8,10 @@ export interface SyncedFetchOnSavedParams { props: SyncedFetchProps; } -export interface SyncedFetchProps - extends Omit, 'get' | 'set'> { +export interface SyncedFetchProps extends Omit< + SyncedOptions, + 'get' | 'set' +> { get: Selector; set?: Selector; getInit?: RequestInit; diff --git a/src/sync-plugins/firebase.ts b/src/sync-plugins/firebase.ts index bdaa9a8e..ee3afe94 100644 --- a/src/sync-plugins/firebase.ts +++ b/src/sync-plugins/firebase.ts @@ -50,7 +50,8 @@ import { clone } from '../globals'; // Should it have mode merge by default? export interface SyncedFirebaseProps - extends Omit, 'list' | 'retry'>, + extends + Omit, 'list' | 'retry'>, Omit, 'onError'> { refPath: (uid: string | undefined) => string; query?: (ref: DatabaseReference) => DatabaseReference | Query; diff --git a/src/sync-plugins/keel.ts b/src/sync-plugins/keel.ts index 1d8d40e7..e2e75ddf 100644 --- a/src/sync-plugins/keel.ts +++ b/src/sync-plugins/keel.ts @@ -82,8 +82,10 @@ interface PageInfo { totalCount: number; } -interface SyncedKeelPropsManyBase - extends Omit, 'list'> { +interface SyncedKeelPropsManyBase extends Omit< + SyncedCrudPropsMany, + 'list' +> { first?: number; get?: never; } @@ -103,8 +105,11 @@ interface SyncedKeelPropsManyWhere< >; where?: Where | (() => Where); } -interface SyncedKeelPropsManyNoWhere - extends SyncedKeelPropsManyBase { +interface SyncedKeelPropsManyNoWhere< + TRemote extends { id: string }, + TLocal, + AOption extends CrudAsOption, +> extends SyncedKeelPropsManyBase { list?: (params: KeelListParams<{}>) => Promise< CrudResult< APIResult<{ @@ -127,8 +132,10 @@ type SyncedKeelPropsMany< ? SyncedKeelPropsManyWhere : SyncedKeelPropsManyNoWhere; -interface SyncedKeelPropsSingle - extends Omit, 'get'> { +interface SyncedKeelPropsSingle extends Omit< + SyncedCrudPropsSingle, + 'get' +> { get?: (params: KeelGetParams) => Promise>; first?: never; @@ -141,11 +148,10 @@ export interface KeelErrorParams extends CrudErrorParams { action: string; } -export interface SyncedKeelPropsBase - extends Omit< - SyncedCrudPropsBase, - 'create' | 'update' | 'delete' | 'updatePartial' | 'fieldUpdatedAt' | 'fieldCreatedAt' | 'onError' - > { +export interface SyncedKeelPropsBase extends Omit< + SyncedCrudPropsBase, + 'create' | 'update' | 'delete' | 'updatePartial' | 'fieldUpdatedAt' | 'fieldCreatedAt' | 'onError' +> { client?: KeelClient; create?: (i: NoInfer>) => Promise>>; update?: (params: { where: any; values?: Partial> }) => Promise>; diff --git a/src/sync-plugins/supabase.ts b/src/sync-plugins/supabase.ts index 8734e629..005aa650 100644 --- a/src/sync-plugins/supabase.ts +++ b/src/sync-plugins/supabase.ts @@ -70,11 +70,10 @@ export type SyncedSupabaseConfig; -export interface SyncedSupabaseConfiguration - extends Omit< - SyncedSupabaseConfig<{ id: string | number }, { id: string | number }>, - 'persist' | keyof SyncedOptions - > { +export interface SyncedSupabaseConfiguration extends Omit< + SyncedSupabaseConfig<{ id: string | number }, { id: string | number }>, + 'persist' | keyof SyncedOptions +> { persist?: SyncedOptionsGlobal; enabled?: Observable; as?: Exclude; @@ -87,8 +86,8 @@ interface SyncedSupabaseProps< TOption extends CrudAsOption = 'object', TRemote extends SupabaseRowOf = SupabaseRowOf, TLocal = TRemote, -> extends SyncedSupabaseConfig, - Omit, 'list'> { +> + extends SyncedSupabaseConfig, Omit, 'list'> { supabase?: Client; collection: Collection; schema?: SchemaName; diff --git a/src/sync-plugins/tanstack-query.ts b/src/sync-plugins/tanstack-query.ts index f5d8acff..4b5afed0 100644 --- a/src/sync-plugins/tanstack-query.ts +++ b/src/sync-plugins/tanstack-query.ts @@ -14,13 +14,17 @@ import { let nextMutationKey = 0; -export interface ObservableQueryOptions - extends Omit, 'queryKey'> { +export interface ObservableQueryOptions extends Omit< + QueryObserverOptions, + 'queryKey' +> { queryKey?: TQueryKey | (() => TQueryKey); } -export interface SyncedQueryParams - extends Omit, 'get' | 'set' | 'retry'> { +export interface SyncedQueryParams extends Omit< + SyncedOptions, + 'get' | 'set' | 'retry' +> { queryClient: QueryClient; query: ObservableQueryOptions; mutation?: MutationObserverOptions; diff --git a/src/sync/syncTypes.ts b/src/sync/syncTypes.ts index 6206dda8..af82c327 100644 --- a/src/sync/syncTypes.ts +++ b/src/sync/syncTypes.ts @@ -104,11 +104,10 @@ export interface SyncedOptions extends Omit void; } -export interface SyncedOptionsGlobal - extends Omit< - SyncedOptions, - 'get' | 'set' | 'persist' | 'initial' | 'waitForSet' | 'waitFor' | 'transform' | 'subscribe' - > { +export interface SyncedOptionsGlobal extends Omit< + SyncedOptions, + 'get' | 'set' | 'persist' | 'initial' | 'waitForSet' | 'waitFor' | 'transform' | 'subscribe' +> { persist?: ObservablePersistPluginOptions & Omit; }