Skip to content

Commit 8430822

Browse files
committed
fix(hooks): accept widened async params; terminal null source on the sync path
Two more items from the second cold review: - Params objects built separately widen async to boolean (and the exported *Params types declare async?: boolean), matching no literal overload — the compiler rejected the call blaming the *source* argument, and values of the hook's own exported param types stopped being accepted. New catch-all overloads (per source type) restore compatibility; literal async values still resolve to the specific overloads first, keeping deprecation warnings and required narrowing. Type-level regression tests gate via yarn tsc. - The sync (non-async) path labeled a null source isLoading: true forever — the exact spinner-hang the async docs warn about. It now settles to a terminal { instance: null, isLoading: false } and, with required: true, throws — mirroring the async path. Behavior change on the deprecated path, in the direction of the documented contract.
1 parent dec621d commit 8430822

3 files changed

Lines changed: 85 additions & 4 deletions

File tree

src/hooks/__tests__/useViewModelInstance.async.test.tsx

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import {
55
waitFor,
66
act,
77
} from '@testing-library/react-native';
8-
import { useViewModelInstance } from '../useViewModelInstance';
8+
import {
9+
useViewModelInstance,
10+
type UseViewModelInstanceFileParams,
11+
} from '../useViewModelInstance';
912
import type { RiveFile } from '../../specs/RiveFile.nitro';
1013
import type { ViewModel, ViewModelInstance } from '../../specs/ViewModel.nitro';
1114
import type { ArtboardBy } from '../../specs/ArtboardBy';
@@ -361,6 +364,40 @@ function createMockRiveViewRef(
361364
return { getViewModelInstance: jest.fn(getViewModelInstance) } as any;
362365
}
363366

367+
describe('useViewModelInstance - param type compatibility', () => {
368+
// These pin overload resolution as much as runtime behavior: a params
369+
// object built separately widens `async` to boolean, and the exported
370+
// params types declare `async?: boolean` — both must remain accepted
371+
// (`yarn tsc` is the gate that fails when the overloads regress).
372+
it('accepts a params object with a widened boolean async', async () => {
373+
const defaultInstance = createMockViewModelInstance();
374+
const defaultViewModel = createMockViewModel({ defaultInstance });
375+
const mockRiveFile = createMockRiveFile({ defaultViewModel });
376+
377+
const params = { async: true }; // widens to { async: boolean }
378+
const { result } = renderHook(() =>
379+
useViewModelInstance(mockRiveFile, params)
380+
);
381+
382+
await waitFor(() => expect(result.current.instance).toBe(defaultInstance));
383+
});
384+
385+
it('accepts a value of the exported file-params type with a nullable source', async () => {
386+
const defaultInstance = createMockViewModelInstance();
387+
const defaultViewModel = createMockViewModel({ defaultInstance });
388+
const mockRiveFile: RiveFile | null | undefined = createMockRiveFile({
389+
defaultViewModel,
390+
});
391+
392+
const params: UseViewModelInstanceFileParams = { async: true };
393+
const { result } = renderHook(() =>
394+
useViewModelInstance(mockRiveFile, params)
395+
);
396+
397+
await waitFor(() => expect(result.current.instance).toBe(defaultInstance));
398+
});
399+
});
400+
364401
describe('useViewModelInstance async - RiveViewRef source', () => {
365402
it('resolves the view-bound instance', async () => {
366403
const vmi = createMockViewModelInstance();

src/hooks/__tests__/useViewModelInstance.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,22 @@ describe('useViewModelInstance - ViewModel source', () => {
363363
});
364364
});
365365

366-
describe('useViewModelInstance - null source', () => {
367-
it('should return undefined instance when source is null', () => {
366+
describe('useViewModelInstance - null vs undefined source', () => {
367+
it('settles to a terminal null when the source is null (e.g. file load failed)', () => {
368+
// Mirrors the async path: `useRiveFile` returns null on error and
369+
// undefined while loading — a null source must not read as isLoading.
368370
const { result } = renderHook(() => useViewModelInstance(null));
369371

372+
expect(result.current.instance).toBeNull();
373+
expect(result.current.isLoading).toBe(false);
374+
expect(result.current.error).toBeNull();
375+
});
376+
377+
it('reports isLoading while the source is undefined (still resolving)', () => {
378+
const { result } = renderHook(() => useViewModelInstance(undefined));
379+
370380
expect(result.current.instance).toBeUndefined();
381+
expect(result.current.isLoading).toBe(true);
371382
expect(result.current.error).toBeNull();
372383
});
373384
});

src/hooks/useViewModelInstance.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export type UseViewModelInstanceRequiredResult =
238238
* during render via deprecated runtime APIs that block the JS thread. The
239239
* async path will become the default in the next major.
240240
*
241-
* With `async: true`, a `null` source settles to a terminal
241+
* In both modes, a `null` source settles to a terminal
242242
* `{ instance: null, isLoading: false }` while an `undefined` source keeps
243243
* the hook loading. This mirrors {@link useRiveFile} (`riveFile: undefined`
244244
* while loading, `null` on error) and `useRive` (`riveViewRef: undefined`
@@ -369,6 +369,25 @@ export function useViewModelInstance(
369369
params?: UseViewModelInstanceRefParams & { async?: false }
370370
): UseViewModelInstanceResult;
371371

372+
// Widened-`async` catch-alls: a params object built separately (or typed
373+
// with the exported *Params types) carries `async?: boolean` and matches no
374+
// literal overload above — without these the compiler rejects the call and
375+
// blames the *source* argument. The flag still must stay constant for the
376+
// component's lifetime. Literal `async` values resolve to the more specific
377+
// overloads first, keeping the deprecation warnings and `required` narrowing.
378+
export function useViewModelInstance(
379+
source: RiveFile | null | undefined,
380+
params?: UseViewModelInstanceFileParams
381+
): UseViewModelInstanceResult;
382+
export function useViewModelInstance(
383+
source: ViewModel | null | undefined,
384+
params?: UseViewModelInstanceViewModelParams
385+
): UseViewModelInstanceResult;
386+
export function useViewModelInstance(
387+
source: RiveViewRef | null | undefined,
388+
params?: UseViewModelInstanceRefParams
389+
): UseViewModelInstanceResult;
390+
372391
// Implementation
373392
export function useViewModelInstance(
374393
source: ViewModelSource | null | undefined,
@@ -443,6 +462,20 @@ function useViewModelInstanceSync(
443462
[result.error]
444463
);
445464

465+
if (result.instance === undefined && source === null) {
466+
// Source resolved to absent/failed rather than pending (`useRiveFile`
467+
// returns null on load error, undefined while loading) — settle to a
468+
// terminal null instead of reporting isLoading forever, mirroring the
469+
// async path.
470+
if (required) {
471+
throw new Error(
472+
'useViewModelInstance: Failed to get ViewModelInstance. ' +
473+
'Ensure the source has a valid ViewModel and instance available.'
474+
);
475+
}
476+
return { instance: null, isLoading: false, error: null };
477+
}
478+
446479
if (required && result.instance === null) {
447480
throw new Error(
448481
result.error

0 commit comments

Comments
 (0)