sindresorhus / type-fest

A collection of essential TypeScript types
Creative Commons Zero v1.0 Universal
14.03k stars 532 forks source link

Allow `SetRequired` to specify internal properties (aka optic) #569

Open Lonli-Lokli opened 1 year ago

Lonli-Lokli commented 1 year ago

Assuming we have

type FormValue = {
  person: {
   city?: string;
  }
}

SetRequired do not allow creation of FormValuee with city set as required. I belive it can be done with template literal types,

eg

type DeepKeyOf<T> = (
  [T] extends [never] ? "" :
  T extends object ? (
    { [K in Exclude<keyof T, symbol>]:
      `${K}${undefined extends T[K] ? "?" : ""}${DotPrefix<DeepKeyOf<T[K]>>}` }[
    Exclude<keyof T, symbol>]
  ) : ""
) extends infer D ? Extract<D, string> : never;

type DotPrefix<T extends string> = T extends "" ? "" : `.${T}`;

Upvote & Fund

Fund with Polar

tommy-mitchell commented 1 year ago

So essentially a SetRequiredDeep type?

Lonli-Lokli commented 1 year ago

I don't think it makes sense to add new one, I think it's better to upgrade existing, as all other code will continue to be valid and type-safe. The only difference is that it will be able to support nested props

Lonli-Lokli commented 1 year ago

Btw I think https://github.com/sindresorhus/type-fest/blob/main/source/set-optional.d.ts can be updated too

Lonli-Lokli commented 1 year ago

@tommy-mitchell I can send PR if you think it's ok

tommy-mitchell commented 1 year ago

That’s up to Sindre, I was just making a suggestion based on the other types in this library.

Lonli-Lokli commented 1 year ago

@sindresorhus what do you think?

sindresorhus commented 1 year ago

So you want SetRequired<Foo, 'person.city'> to work, correct? That would be a breaking change as it's ambiguous whether person.city is a key path or just a key name. person.city is a perfectly valid key name.

I think it would be better to make a SetRequiredDeep that by default makes all keys recursively required and if you specify a key path, it makes a specific deep key required. @tommy-mitchell What do you think?

tommy-mitchell commented 1 year ago

I think it would be better to make a SetRequiredDeep that by default makes all keys recursively required and if you specify a key path, it makes a specific deep key required.

I think that fits with the rest of type-fest and has an intuitive usage.

Lonli-Lokli commented 1 year ago

So you want SetRequired<Foo, 'person.city'> to work, correct? That would be a breaking change as it's ambiguous whether person.city is a key path or just a key name. person.city is a perfectly valid key name.

I think it would be better to make a SetRequiredDeep that by default makes all keys recursively required and if you specify a key path, it makes a specific deep key required. @tommy-mitchell What do you think?

Actually I think it's very rare case to have both


type FormValueCombined = {
  person: {
   city: string;
  },
  ['person.city']: number;
}

But anyway it can be constructed such way that top-level property will have high priority for backward-compatibility.

If you mean that user would not be able to have dot inside the name so no, it will be still valid to have dots in names

Lonli-Lokli commented 1 year ago

So here is draft playground for keys https://tsplay.dev/Wzky4w

sindresorhus commented 1 year ago

@Lonli-Lokli Thanks for elaborating. My only concern left for having it in SetRequried is that it may affect type-checking performance. We had this problem in Simplify and had to split the deep handling (it even affected non-deep usage) into a separate type.

sindresorhus commented 1 year ago

So here is draft playground for keys https://tsplay.dev/Wzky4w

The key path handling is similar to https://github.com/sindresorhus/type-fest/pull/409/files and Get. Would be nice to somehow unify all that.

tommy-mitchell commented 1 year ago

Would be nice to somehow unify all that.

I've been working on a Paths type

Lonli-Lokli commented 1 year ago

I will just leavee it here as a some kind of sample

type StringOrNumKeys<TObj> = keyof TObj & (string | number);

type NestedPath<TValue, Prefix extends string> =
    TValue extends object
    ? `${Prefix}.${NestedFieldPaths<TValue>}`
    : never;

export type NestedFieldPaths<TData> = {
    [TKey in StringOrNumKeys<TData>]: `${TKey}` | NestedPath<TData[TKey], `${TKey}`>;
}[StringOrNumKeys<TData>];
Lonli-Lokli commented 1 year ago

Let me up this issue, is there any plans to implement it in the nearest future?