Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Dot Notation key support to utility functions #2256

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Dot Notation key support to utility functions #2256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/foundry/common/utils/helpers.mjs.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import type { TypeOfTag } from "typescript/lib/typescript";
import type {
DotNotationKeys,
DotNotationObject,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As GitHub Actions lints this import is unused.

FlatObject,
GetValueFromDotKey,
InputValue
} from "../../../types/helperTypes";

/**
* Benchmark the performance of a function, calling it a requested number of iterations.
Expand Down Expand Up @@ -172,6 +179,7 @@ export declare function encodeURL(path: string): string;
* (default: `0`)
* @returns An expanded object
*/
export declare function expandObject<T extends Record<string, unknown>>(obj: FlatObject<T>, _d?: number): T;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inference here isn't going to be ideal here, I think, based upon my prior experience with "inverse inference" -- where to infer T you have to take what obj is and apply the inverse of FlatObject which is obviously ExpandObject here. While on paper it's something the compiler could attempt to do, T is probably going to be inferred overly broad as like Record<string, unknown> or something. Here's some test cases you can add to try it out:

expectType<{ foo: { bar: 1 } }>(expandObject({ "foo.bar": 1 }));
expectType<{ foo: { bar: 1 } }>(expandObject({ "foo.bar": 1, "foo.lorem": 2 }));
expectType<{ foo: { bar: 1, lorem: 2 } }>(expandObject({ "foo": { "bar": 1 }, "foo.lorem": "2" }));

We can get this draft in first and I can add in an ExpandObject helper type later if so desired as this is strictly an improvement.

export declare function expandObject(obj: object, _d?: number): any;

/**
Expand Down Expand Up @@ -213,6 +221,7 @@ interface FilterObjectOptions {
* @param d - Track the recursion depth to prevent overflow
* @returns A flattened object
*/
export declare function flattenObject<T extends Record<string, unknown>>(obj: T, _d?: number): FlatObject<T>;
export declare function flattenObject(obj: object, _d?: number): any;

/**
Expand Down Expand Up @@ -256,7 +265,7 @@ export function getType(
* @param key - An object property with notation a.b.c
* @returns An indicator for whether the property exists
*/
export declare function hasProperty(object: object, key: string): boolean;
export declare function hasProperty<T extends InputValue, key extends DotNotationKeys<T>>(object: T, key: key): boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit; type parameters in this repo should be PascalCase so it should be Key extends DotNotationKeys<T>


/**
* A helper function which searches through an object to retrieve a value by a string key.
Expand All @@ -265,7 +274,10 @@ export declare function hasProperty(object: object, key: string): boolean;
* @param key - An object property with notation a.b.c
* @returns The value of the found property
*/
export declare function getProperty(object: object, key: string): any;
export declare function getProperty<T extends InputValue, K extends DotNotationKeys<T>>(
object: T,
key: K
): GetValueFromDotKey<T, K>;

/**
* A helper function which searches through an object to assign a value using a string key
Expand All @@ -275,7 +287,11 @@ export declare function getProperty(object: object, key: string): any;
* @param value - The value to be assigned
* @returns Whether the value was changed from its previous value
*/
export declare function setProperty(object: object, key: string, value: any): boolean;
export declare function setProperty<T extends InputValue, K extends DotNotationKeys<T>>(
object: T,
key: K,
value: GetValueFromDotKey<T, K>
): boolean;

/**
* Invert an object by assigning its values as keys and its keys as values.
Expand Down
92 changes: 92 additions & 0 deletions src/types/helperTypes.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,95 @@ export type DataSourceForPlaceable<P extends PlaceableObject> = P extends Placea
? D["_source"]
: never
: never;

type EndValue = string | bigint | number | boolean | null | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this PrimitiveValues as that seems to be a more descriptive name. Though if it's meant to be any primitive value it should be symbol as well.

export type InputValue = object | Array<unknown>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object | Array<unknown> is redundant as object includes arrays. I'd do Record<string, unknown> | Array<unknown> for essentially clarity. Also I'd probably not make this exported.


type Split<S extends string, D extends string> = string extends S
? string[]
: S extends ""
? []
: S extends `${infer T}${D}${infer U}`
? [T, ...Split<U, D>]
: [S];

type JoinParts<T extends (string | number)[], D extends string> = T extends []
? never
: T extends [infer F]
? `${F & (number | string)}`
: T extends [infer F, ...infer R]
? F extends EndValue
? `${F}${D}${JoinParts<Extract<R, (string | number)[]>, D>}`
: never
: string;
// TODO: fix for TS4.8 for tuples, string to number literal cast
export type ArrayOrTupleKey<T> = StringNumber<keyof T & string> extends never ? number : StringNumber<keyof T & string>;

type GetPathParts<T, LeafsOnly extends boolean = false> = T extends EndValue
? []
: Exclude<
T extends Array<unknown>
?
| {
[K in ArrayOrTupleKey<T>]: [K, ...GetPathParts<T[K], LeafsOnly>];
}[ArrayOrTupleKey<T>]
| (LeafsOnly extends true ? never : [ArrayOrTupleKey<T>])
:
| {
[K in Extract<keyof T, EndValue>]: [K, ...GetPathParts<T[K], LeafsOnly>];
}[Extract<keyof T, EndValue>]
| (LeafsOnly extends true ? never : [Extract<keyof T, EndValue>]),
never
>;

type GetPathPartsObjectOnly<T extends object> = T extends EndValue
? []
: Exclude<
{
[K in Extract<keyof T, EndValue>]: [K, ...GetPathParts<T[K]>];
}[Extract<keyof T, EndValue>],
never
>;

export type GetValueFromDotKey<T, P extends string> = InnerGetValueFromDotKey<T, Split<P, ".">>;
type InnerGetValueFromDotKey<T, P extends string[]> = T extends Array<unknown>
? InnerGetValueFromDotKeyArray<T, P>
: T extends object
? InnerGetValueFromDotKeyObject<T, P>
: never;
// TODO: fix for TS4.8 for tuples, string to number literal cast
type InnerGetValueFromDotKeyArray<T extends Array<unknown>, P extends string[]> = P extends [string]
? T[number]
: P extends [string, ...infer R]
? InnerGetValueFromDotKey<T[number], Extract<R, string[]>>
: never;
type InnerGetValueFromDotKeyObject<T extends object, P extends string[]> = P extends [infer K]
? T[K & keyof T]
: P extends [keyof T, ...infer R]
? InnerGetValueFromDotKey<T[P[0] & keyof T], Extract<R, string[]>>
: never;

/** @internal exported for tests */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say you should just test this by checking DotNotationObject's keys or something. If you're thoroughly testing DotNotationObject it shouldn't matter anyways.

export type DotNotationKeys<T extends InputValue, LeafsOnly extends boolean = false> = JoinParts<
GetPathParts<T, LeafsOnly>,
"."
>;

export type DotNotationObject<T extends InputValue> = {
[K in DotNotationKeys<T>]: GetValueFromDotKey<T, K>;
};

export type FlatObject<T extends Record<string, unknown>> = {
[K in JoinParts<GetPathPartsObjectOnly<T>, ".">]: GetValueFromDotKey<T, K>;
};

type AddDeletionMarkToLastPart<T> = T extends [...infer L, infer B]
? L extends (string | number)[]
? [...L, `-=${B & (string | number)}`]
: never
: never;
/** @internal exported for tests */
export type DeleteKeys<T> = JoinParts<AddDeletionMarkToLastPart<GetPathParts<T>>, ".">;
export type DeletionUpdate<T extends InputValue> = {
[K in DeleteKeys<T> | DotNotationKeys<T>]?: K extends DeleteKeys<T> ? unknown : GetValueFromDotKey<T, K>;
};
2 changes: 2 additions & 0 deletions src/types/utils.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,5 @@ type PropertyTypeOrFallback<T, Key extends string, Fallback> = Key extends keyof
* Makes the given keys `K` of the type `T` required
*/
type RequiredProps<T, K extends keyof T> = Required<Pick<T, K>> & Omit<T, K>;

type StringNumber<S extends string> = S extends `${number}` ? S : never;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call this NumberToString, on the first usage I saw, I sort thought it was the inverse; a string to a number.

38 changes: 38 additions & 0 deletions test-d/foundry/common/utils/helpers.mjs.test-d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,44 @@ declare class ClassWithConstructorParameters {
expectType<boolean>(foundry.utils.isSubclass(ClassWithNoConstructorParameters, ClassWithConstructorParameters));
expectType<boolean>(foundry.utils.isSubclass(ClassWithConstructorParameters, ClassWithNoConstructorParameters));

// expandObject
// hard to infer back
expectType<{}>(foundry.utils.expandObject({}));
expectType<{ test: number }>(foundry.utils.expandObject({ test: 1 }));
expectType<{ test: number; deep: { value: number } }>(
foundry.utils.expandObject<{ test: number; deep: { value: number } }>({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly this test isn't really testing anything to do with expandObject itself as a function, it's testing that the first generic parameter is the return Ideally we wouldn't even make these types of generic parameters visible, it's just a way to access the parameters when getting the output. I would just remove it as this isn't a desired way to see it used. The inference will obviously be wrong but I can update this with an ExpandObject type.

test: 1,
"deep.value": 1
})
);
expectType<{ "0": number }>(foundry.utils.expandObject<{ "0": number }>({ "0": 1 }));

// hasProperty
expectType<boolean>(foundry.utils.hasProperty({ k1: 1 }, "k1"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just note that hasProperty is fundamentally not as ideal as it could in Typescript. It can't really be expressed as a type guard without running into a million unions.

expectError(foundry.utils.hasProperty({ k1: 1 }, "k2"));
expectType<boolean>(foundry.utils.hasProperty({ deep: { value: 1 } }, "deep"));
expectType<boolean>(foundry.utils.hasProperty({ deep: { value: 1 } }, "deep.value"));
expectError(foundry.utils.hasProperty({ deep: { key: 1 } }, "deep.value"));

// getProperty
expectType<number>(foundry.utils.getProperty({ k1: 1, k2: "a" }, "k1"));
expectType<string>(foundry.utils.getProperty({ k1: 1, k2: "a" }, "k2"));
expectError(foundry.utils.getProperty({ k1: 1, k2: "a" }, "k3"));
expectType<{ value: number }>(foundry.utils.getProperty({ deep: { value: 1 } }, "deep"));
expectType<number>(foundry.utils.getProperty({ deep: { value: 1 } }, "deep.value"));
expectType<1>(foundry.utils.getProperty({ deep: { value: 1 as const } }, "deep.value"));

// setProperty
expectType<boolean>(foundry.utils.setProperty({ k1: 1, k2: "a" }, "k1", 2));
expectType<boolean>(foundry.utils.setProperty({ k1: 1, k2: "a" }, "k2", "b"));
expectError(foundry.utils.setProperty({ k1: 1, k2: "a" }, "k3", null));
expectType<boolean>(foundry.utils.setProperty({ deep: { value: 0 } }, "deep", { value: 1 }));
expectType<boolean>(foundry.utils.setProperty({ deep: { value: 0 } }, "deep.value", 1));
expectType<boolean>(foundry.utils.setProperty({ deep: { value: 1 as 0 | 1 | -1 } }, "deep.value", -1));
expectError(foundry.utils.setProperty({ deep: { value: 1 as 0 | 1 | -1 } }, "deep.value", 2));
expectType<boolean>(foundry.utils.setProperty({ other: 1 } as Record<string, number>, "test", 3));
expectType<boolean>(foundry.utils.setProperty({ other: 1 } as Record<"test" | "other", number>, "test", 3));

// invertObject
expectType<{ readonly 1: "a"; readonly foo: "b" }>(foundry.utils.invertObject({ a: 1, b: "foo" } as const));

Expand Down
63 changes: 63 additions & 0 deletions test-d/types/helperTypes.test-d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { expectType } from "tsd";
import type { ArrayOrTupleKey, DotNotationKeys, GetValueFromDotKey } from "../../src/types/helperTypes";

declare function type<T>(): T;

// basic objects
expectType<"k1">(type<DotNotationKeys<{ k1: 1 }>>());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's not a bad idea to start testing our types I think the type function helper here is clumsy.
Try this:

import { TypeEqual } from "ts-expect";

expectType<TypeEqual<DotNotationKeys{ k1: 1 }>, "k1">(true);

It's still a a bit awkward but it's at least more expressive and something shown in tsd's examples.

expectType<"k2">(type<DotNotationKeys<{ k2: 1 }>>());
expectType<"k1" | "k2">(type<DotNotationKeys<{ k1: 1; k2: 1 }>>());

// objects with depth
expectType<"k1" | "k2" | "deep" | "deep.value">(type<DotNotationKeys<{ k1: 1; k2: 1; deep: { value: 1 } }>>());
expectType<
| "a"
| "a.really"
| "a.really.deep"
| "a.really.deep.object"
| "a.really.deep.object.holding"
| "a.really.deep.object.holding.a"
| "a.really.deep.object.holding.a.value"
>(type<DotNotationKeys<{ a: { really: { deep: { object: { holding: { a: { value: 1 } } } } } } }>>());

// arrays
expectType<`${number}`>(type<DotNotationKeys<number[]>>());
expectType<`${number}` | `${number}.${number}`>(type<DotNotationKeys<number[][]>>());
expectType<`${number}` | `${number}.${number}` | `${number}.${number}.${number}`>(
type<DotNotationKeys<number[][][]>>()
);

// tuples!
expectType<"0">(type<ArrayOrTupleKey<[9]>>());
expectType<"0" | "1">(type<ArrayOrTupleKey<[9, 9]>>());
expectType<number>(type<ArrayOrTupleKey<9[]>>());
expectType<"0">(type<DotNotationKeys<[9]>>());
expectType<"0" | "1">(type<DotNotationKeys<[9, 9]>>());
expectType<"0" | "0.int" | "0.str">(type<DotNotationKeys<[{ int: 1; str: "" }]>>());
expectType<"0" | "0.int" | "0.str" | "1">(type<DotNotationKeys<[{ int: 1; str: "" }, 2]>>());

// arrays with objects
expectType<`${number}` | `${number}.k1`>(type<DotNotationKeys<{ k1: 1 }[]>>());
expectType<`${number}` | `${number}.k2`>(type<DotNotationKeys<{ k2: 2 }[]>>());
expectType<`${number}` | `${number}.k1` | `${number}.k2`>(type<DotNotationKeys<{ k1: 1; k2: 2 }[]>>());

// objects with arrays
expectType<`array` | `array.${number}`>(type<DotNotationKeys<{ array: number[] }>>());
expectType<`array` | `array.${number}` | `deep` | `deep.array` | `deep.array.${number}`>(
type<DotNotationKeys<{ array: number[]; deep: { array: number[] } }>>()
);

// objects with tuples
expectType<"tuple" | "tuple.0" | "tuple.1" | "tuple.0.name" | "tuple.1.name">(
type<DotNotationKeys<{ tuple: [{ name: string }, { name: string }] }>>()
);

// mixed
expectType<"tuple" | "tuple.0" | "tuple.1" | "tuple.2" | "array" | `array.${number}` | "deep" | "deep.value">(
type<DotNotationKeys<{ tuple: [1, 2, 3]; array: number[]; deep: { value: number } }>>()
);
expectType<"0" | "1" | `1.${number}` | "2" | "2.value">(type<DotNotationKeys<[1, number[], { value: number }]>>());
expectType<`${number}` | `${number}.0` | `${number}.1`>(type<DotNotationKeys<[number, number][]>>());

expectType<number>(type<GetValueFromDotKey<{ value: number }, "value">>());
expectType<number>(type<GetValueFromDotKey<[number, 1], "0">>());