Skip to content

Commit

Permalink
Merge pull request #528 from thejustinwalsh/use-assets-types-bug
Browse files Browse the repository at this point in the history
fix: ensure that useAssets does not infer type from data
  • Loading branch information
trezy authored Aug 23, 2024
2 parents 1c4e6c3 + 1427db4 commit ade982d
Show file tree
Hide file tree
Showing 7 changed files with 339 additions and 90 deletions.
297 changes: 214 additions & 83 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"@rollup/plugin-commonjs": "^25.0.8",
"@rollup/plugin-json": "^6.1.0",
"@rollup/plugin-node-resolve": "^15.2.3",
"@testing-library/react": "^16.0.0",
"@types/eslint": "^8.56.10",
"@types/react": "^18.3.2",
"@types/react-reconciler": "0.28.8",
Expand All @@ -90,6 +91,9 @@
"optional": true
}
},
"overrides": {
"rollup": "^4.18.0"
},
"publishConfig": {
"access": "public"
},
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useAsset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import type { ErrorCallback } from '../typedefs/ErrorCallback';
const errorCache: Map<UnresolvedAsset | string, AssetRetryState> = new Map();

/** @deprecated Use `useAssets` instead. */
export function useAsset<T>(
export function useAsset<T = any>(
/** @description Asset options. */
options: (UnresolvedAsset<T> & AssetRetryOptions) | string,
options: (UnresolvedAsset & AssetRetryOptions) | string,
/** @description A function to be called when the asset loader reports loading progress. */
onProgress?: ProgressCallback,
/** @description A function to be called when the asset loader reports loading progress. */
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ function assetsLoadedTest<T>(asset: UnresolvedAsset<T>)
/** Loads assets, returning a hash of assets once they're loaded. */
export function useAssets<T = any>(
/** @description Assets to be loaded. */
assets: UnresolvedAsset<T>[],
assets: UnresolvedAsset[],

/** @description Asset options. */
options: UseAssetsOptions = {},
): UseAssetsResult<T>
{
const [state, setState] = useState<UseAssetsResult<T>>({
assets: Array(assets.length).fill(null),
assets: Array(assets.length).fill(undefined),
isError: false,
isPending: true,
isSuccess: false,
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useSuspenseAssets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function assetsLoadedTest<T>(asset: UnresolvedAsset<T>)
/** Loads assets, returning a hash of assets once they're loaded. Must be inside of a `<Suspense>` component. */
export function useSuspenseAssets<T = any>(
/** @description Assets to be loaded. */
assets: UnresolvedAsset<T>[],
assets: UnresolvedAsset[],
/** @description Asset options. */
options: UseAssetsOptions = {},
): T[]
Expand Down
4 changes: 2 additions & 2 deletions src/typedefs/UseAssetsResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import type { UseAssetsStatus } from '../constants/UseAssetsStatus';

export interface UseAssetsResult<T>
{
/** @description An array of resolved assets, or `null` for assets that are still loading. */
assets: (T | null)[];
/** @description An array of resolved assets, or `undefined` for assets that are still loading. */
assets: (T | undefined)[];

/** @description The error that was encountered. */
error?: Error;
Expand Down
114 changes: 114 additions & 0 deletions test/unit/hooks/useAssets.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { Assets, Cache, Sprite, Texture, type UnresolvedAsset } from 'pixi.js';
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { extend, useAssets } from '../../../src';
import { cleanup, renderHook, waitFor } from '@testing-library/react';

extend({ Sprite });

describe('useAssets', async () =>
{
const assets: UnresolvedAsset[] = [{ src: 'test.png' }, { src: 'test2.png' }];

// Store the loaded assets and data state to verify the hook results
let loaded: Record<string, unknown> = {};
let data: Record<string, any> = {};

// Mock the Assets.load, Assets.get & Cache.has method
const load = vi.spyOn(Assets, 'load');
const get = vi.spyOn(Assets, 'get');
const has = vi.spyOn(Cache, 'has');

// Mock the Assets.load to populate the loaded record, and resolve after 1ms
load.mockImplementation((urls) =>
{
const assets = urls as UnresolvedAsset[];

return new Promise((resolve) =>
{
setTimeout(() =>
{
loaded = { ...loaded, ...assets.reduce((acc, val) => ({ ...acc, [val.src!.toString()]: Texture.EMPTY }), {}) };
data = { ...data, ...assets.reduce((acc, val) => ({ ...acc, [val.src!.toString()]: val.data }), {}) };
resolve(loaded);
}, 1);
});
});

// Mock the Assets.get to return the loaded record
get.mockImplementation((keys) =>
keys.reduce<Record<string, unknown>>((acc, key, idx) => ({ ...acc, [idx]: loaded[key] }), {}));

// Mock the Cache.has to check if the key is in the loaded record
has.mockImplementation((key) => key in loaded);

// Load the default results using Assets.load to compare against the results from the useAssets hook
const defaultResults = await Assets.load<Texture>(assets);

beforeEach(() =>
{
loaded = {};
data = {};
});

afterEach(() =>
{
cleanup();
});

afterAll(() =>
{
load.mockRestore();
get.mockRestore();
});

it('loads assets', async () =>
{
const { result } = renderHook(() => useAssets<Texture>(assets));

expect(result.current.isPending).toBe(true);
await waitFor(() => expect(result.current.isSuccess).toBe(true));

expect(result.current.assets).toEqual(assets.map(({ src }) => defaultResults[src!.toString()]));
});

it('accepts data', async () =>
{
// Explicitly type the T in the useAssets hook
const { result } = renderHook(() => useAssets<Texture>([
{ src: 'test.png', data: { test: '7a1c8bee' } },
{ src: 'test2.png', data: { test: '230a3f41' } },
]));

expect(result.current.isPending).toBe(true);
await waitFor(() => expect(result.current.isSuccess).toBe(true));

const { assets: [texture], isSuccess } = result.current;

expect(isSuccess).toBe(true);
expect(data['test.png'].test).toBe('7a1c8bee');
expect(data['test2.png'].test).toBe('230a3f41');

const isTexture = (texture?: Texture) => texture && texture instanceof Texture;

expect(isTexture(texture)).toBe(true);
});

it('is properly typed with data', async () =>
{
// Do not provide a type for T in the useAssets hook
const { result } = renderHook(() => useAssets([
{ src: 'test.png', data: { test: 'd460dbdd' } },
]));

expect(result.current.isPending).toBe(true);
await waitFor(() => expect(result.current.isSuccess).toBe(true));

const { assets: [texture], isSuccess } = result.current;

expect(isSuccess).toBe(true);

const isTexture = (texture?: Texture) => texture && texture instanceof Texture;

expect(isTexture(texture)).toBe(true);
});
});

0 comments on commit ade982d

Please sign in to comment.