Repository navigation
Ensure ref passed to native elements are narrowly type checked #49
Description
Activity
According to DeepSeek V4 Flash 0731, fixing vobyjs/oby#14 also fixes this issue.
The problem with (): T
In Voby, a ref is typed as:
type Ref<T> = (value: T) => voidObservables are callable, so this works at runtime:
const inputRef = $<HTMLInputElement>()
<input ref={inputRef} />When TypeScript checks this, it tries to assign the observable to:
(value: HTMLInputElement) => voidThe problem is that an observable has multiple call signatures:
type Observable<T> = {
(): T
(fn: (value: T) => T): T
(value: T): T
}TypeScript only needs one of these signatures to be compatible with the target.
The getter (): T causes the problem.
A function with no parameters can be assigned to a function that accepts a parameter because JavaScript allows the extra argument to be ignored. Therefore (): T is considered compatible with:
(value: HTMLInputElement) => voidThe actual type of T doesn't matter.
That means this incorrectly passes:
const buttonRef = $<HTMLButtonElement | null>()
<input ref={buttonRef} />The getter signature satisfies the Ref<HTMLInputElement> check, so TypeScript never needs to use the setter signature to verify that the ref can actually accept an HTMLInputElement.
In other words, the ref type check is effectively bypassed.
Why (...args: never[]) fixes it
Change the getter from (): T to (...args: never[]): T.
Now the getter cannot match:
(value: HTMLInputElement) => voidbecause the target passes an HTMLInputElement, but the getter only accepts never.
The getter can no longer bypass the check. TypeScript has to use the setter:
(value: T): TWith strictFunctionTypes, this is checked contravariantly, so the ref only works when its value type can actually accept the element type.
For example:
const inputRef = $<HTMLInputElement>()
<input ref={inputRef} /> // ✓
const buttonRef = $<HTMLButtonElement | null>()
<input ref={buttonRef} /> // ✗Changes
diff --git a/src/methods/readonly.ts b/src/methods/readonly.ts
--- a/src/methods/readonly.ts
+++ b/src/methods/readonly.ts
@@ -10,7 +10,7 @@ import type {Observable, ObservableReadonly} from '~/types';
const readonly = <T> ( observable: Observable<T> | ObservableReadonly<T> ): ObservableReadonly<T> => {
- if ( isObservableWritable ( observable ) ) {
+ if ( isObservableWritable<T> ( observable ) ) {
return readable ( target ( observable ) );
diff --git a/src/methods/store.ts b/src/methods/store.ts
--- a/src/methods/store.ts
+++ b/src/methods/store.ts
@@ -12,7 +12,7 @@ import {readable} from '~/objects/callable';
import ObservableClass from '~/objects/observable';
import {SYMBOL_STORE, SYMBOL_STORE_KEYS, SYMBOL_STORE_OBSERVABLE, SYMBOL_STORE_TARGET, SYMBOL_STORE_VALUES, SYMBOL_STORE_UNTRACKED} from '~/symbols';
import {castArray, is, isArray, isFunction, isObject, noop, nope} from '~/utils';
-import type {IObservable, CallbackFunction, DisposeFunction, EqualsFunction, Observable, ObservableOptions, StoreOptions, ArrayMaybe, LazySet} from '~/types';
+import type {IObservable, CallbackFunction, DisposeFunction, EqualsFunction, Observable, ObservableOptions, ObservableReadonly, StoreOptions, ArrayMaybe, LazySet} from '~/types';
/* TYPES */
@@ -271,7 +271,7 @@ const STORE_TRAPS = {
if ( key === SYMBOL_STORE_OBSERVABLE ) {
- return ( key: StoreKey ): Observable<unknown> => {
+ return ( key: StoreKey ): ObservableReadonly<unknown> => {
key = ( typeof key === 'number' ) ? String ( key ) : key;
diff --git a/src/objects/callable.ts b/src/objects/callable.ts
--- a/src/objects/callable.ts
+++ b/src/objects/callable.ts
@@ -53,7 +53,7 @@ const readable = <T> ( value: IObservable<T> ): ObservableReadonly<T> => {
};
const writable = <T> ( value: IObservable<T> ): Observable<T> => {
- const fn = writableFunction.bind ( value as any ) as ObservableReadonly<T>; //TSC
+ const fn = writableFunction.bind ( value as any ) as unknown as Observable<T>; //TSC
fn[SYMBOL_OBSERVABLE] = true;
fn[SYMBOL_OBSERVABLE_WRITABLE] = value;
return fn;
diff --git a/src/types.ts b/src/types.ts
--- a/src/types.ts
+++ b/src/types.ts
@@ -91,25 +91,25 @@ type MemoOptions<T = unknown> = {
/* OBSERVABLE */
type Observable<T = unknown> = {
- (): T,
+ ( ...args: never[] ): T,
( fn: ( value: T ) => T ): T,
( value: T ): T,
readonly [ObservableSymbol]: true
};
type ObservableLike<T = unknown> = {
- (): T,
+ ( ...args: never[] ): T,
( fn: ( value: T ) => T ): T,
( value: T ): T
};
type ObservableReadonly<T = unknown> = {
- (): T,
+ ( ...args: never[] ): T,
readonly [ObservableSymbol]: true
};
type ObservableReadonlyLike<T = unknown> = {
- (): T
+ ( ...args: never[] ): T
};
type ObservableOptions<T = unknown> = {
diff --git a/type-tests/index.ts b/type-tests/index.ts
new file mode 100644
--- /dev/null
+++ b/type-tests/index.ts
@@ -0,0 +1,33 @@
+/* IMPORT */
+
+import type {Observable, ObservableLike, ObservableReadonly, ObservableReadonlyLike} from '../src/types';
+
+/* MAIN */
+
+// "Observable" means "ObservableWritable": a readonly Observable must NOT be
+// assignable to a writable one, otherwise writes sneak through a readonly view.
+
+declare const writable: Observable<number>;
+declare const readonly: ObservableReadonly<number>;
+declare const writableLike: ObservableLike<number>;
+declare const readonlyLike: ObservableReadonlyLike<number>;
+
+// @ts-expect-error ObservableReadonly must not be assignable to Observable
+const shouldFail1: Observable<number> = readonly;
+// @ts-expect-error ObservableReadonly like must not be assignable to ObservableLike
+const shouldFail2: ObservableLike<number> = readonlyLike;
+// @ts-expect-error ObservableReadonlyLike must not be assignable to Observable
+const shouldFail3: Observable<number> = readonlyLike;
+
+// The reverse direction stays valid: a writable Observable IS a readonly one.
+
+const readable: ObservableReadonly<number> = writable;
+const readableLike: ObservableReadonlyLike<number> = readonly;
+const readableLike2: ObservableReadonlyLike<number> = writable;
+
+void shouldFail1;
+void shouldFail2;
+void shouldFail3;
+void readable;
+void readableLike;
+void readableLike2;
\ No newline at end of filediff --git a/src/types.ts b/src/types.ts
--- a/src/types.ts
+++ b/src/types.ts
@@ -131,9 +131,9 @@ type StoreOptions = import ( 'oby' ).StoreOptions;
type Styles = FunctionMaybe<null | undefined | string | Record<string, FunctionMaybe<null | undefined | number | string>> | (FunctionMaybe<null | undefined | number | string> | Styles)[]>;
-type SuspenseCollectorData = { active: Observable<boolean>, register: ( suspense: SuspenseData ) => void, unregister: ( suspense: SuspenseData ) => void };
+type SuspenseCollectorData = { active: ObservableReadonly<boolean>, register: ( suspense: SuspenseData ) => void, unregister: ( suspense: SuspenseData ) => void };
-type SuspenseData = { active: Observable<boolean>, increment: ( nr?: number ) => void, decrement: ( nr?: number ) => void };
+type SuspenseData = { active: ObservableReadonly<boolean>, increment: ( nr?: number ) => void, decrement: ( nr?: number ) => void };
type TemplateActionPath = number[];
diff --git a/src/utils/setters.ts b/src/utils/setters.ts
--- a/src/utils/setters.ts
+++ b/src/utils/setters.ts
@@ -695,7 +695,7 @@ const setEventStatic = (() => {
})();
-const setEvent = ( element: HTMLElement, event: string, value: ObservableMaybe<null | undefined | EventListener> ): void => {
+const setEvent = ( element: HTMLElement, event: string, value: null | undefined | EventListener ): void => {
setEventStatic ( element, event, value );Why setEvent also changes
There is a separate, smaller cleanup in setters.ts.
setEvent receives an already-resolved event handler. Reactive values are handled earlier by setProp and setTemplateAccessor, so an observable or thunk should never reach setEvent.
Therefore this:
const setEvent = (
element: HTMLElement,
event: string,
value: ObservableMaybe<null | undefined | EventListener>
): voidis misleading.
The function actually expects:
const setEvent = (
element: HTMLElement,
event: string,
value: null | undefined | EventListener
): voidThis also matches setEventStatic, which already uses the concrete type.
The public JSX event props should still accept ObservableMaybe<...> because those values are resolved before reaching setEvent.
This should be a type error: