Skip to content

Commit

Permalink
Fix class defines as property versus method in Collection
Browse files Browse the repository at this point in the history
  • Loading branch information
LukeAbby committed Jan 3, 2025
1 parent a2ad39c commit 2bc9a91
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 23 deletions.
28 changes: 7 additions & 21 deletions src/foundry/common/utils/collection.d.mts
Original file line number Diff line number Diff line change
@@ -1,31 +1,17 @@
interface MapReplacementMembers<K, V> {
set(key: K, value: V): this;
// This class exists make it as sound as possible to override these parts of the class and make them
// completely unrelated. It's done this way specifically to avoid situations like
declare class LenientMap<K, V> extends Map<K, V> {
[Symbol.iterator](): any;
forEach(...args: any[]): any;
}

type PatchedMap<K, V> = Omit<Map<K, V>, "forEach" | typeof Symbol.iterator | "get" | "set"> &
MapReplacementMembers<K, V>;

declare namespace PatchedMap {
type Any = PatchedMap<any, any>;
}

interface PatchedMapConstructor {
new (): PatchedMap.Any;
new <K, V>(entries?: readonly (readonly [K, V])[] | null): PatchedMap<K, V>;
new <K, V>(iterable: Iterable<readonly [K, V]>): PatchedMap<K, V>;
readonly [Symbol.species]: PatchedMapConstructor;
readonly prototype: PatchedMap.Any;
}

declare const Map: PatchedMapConstructor;

/**
* A reusable storage concept which blends the functionality of an Array with the efficient key-based lookup of a Map.
* This concept is reused throughout Foundry VTT where a collection of uniquely identified elements is required.
* @typeParam T - The type of the objects contained in the Collection
*/
declare class Collection<V> extends Map<string, V> {
constructor(entries?: readonly (readonly [string, V])[] | null);
declare class Collection<V> extends LenientMap<string, V> {
constructor(entries?: Iterable<readonly [string, V]> | null);

/**
* When iterating over a Collection, we should iterate over its values instead of over its entries
Expand Down
21 changes: 19 additions & 2 deletions tests/foundry/common/utils/collection.mjs.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,25 @@ function isString(e: string | null): e is string {
}

const cn = new Collection<string | null>();
expectTypeOf(cn.filter((each) => typeof each === "string")).toEqualTypeOf<Array<string | null>>();
expectTypeOf(cn.find((each) => typeof each === "string")).toEqualTypeOf<string | null | undefined>();
expectTypeOf(cn.filter((each) => typeof each === "string")).toEqualTypeOf<Array<string>>();
expectTypeOf(cn.find((each) => typeof each === "string")).toEqualTypeOf<string | undefined>();

expectTypeOf(cn.filter(isString)).toEqualTypeOf<string[]>();
expectTypeOf(cn.find(isString)).toEqualTypeOf<string | undefined>();

// This is a regression test for the error:
// Class '...' defines instance member property '...', but extended class '...' defines it as instance member function.
// This occurred because of how `Map` was patched to allow the unsound subclassing of `Collection`.
class CustomCollection extends Collection<string> {
override clear(): void {}

override delete(_key: string): boolean {
return true;
}
}

declare const customCollection: CustomCollection;

if (customCollection instanceof Map) {
expectTypeOf(customCollection).toEqualTypeOf<CustomCollection>();
}

0 comments on commit 2bc9a91

Please sign in to comment.