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.

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

Already on GitHub? Sign in to your account

O.P.Pick from arrays #57

Closed
mcpower opened this issue Sep 30, 2019 · 17 comments 路 Fixed by #81
Closed

O.P.Pick from arrays #57

mcpower opened this issue Sep 30, 2019 · 17 comments 路 Fixed by #81
Assignees
Labels
feature Add new features wontfix This will not be worked on

Comments

@mcpower
Copy link
Contributor

mcpower commented Sep 30, 2019

馃崺 Feature Request

Is your feature request related to a problem?

In the APIs I'm working with, I have some nested properties - for example:

type Household = {
  id: number
  people: {
    name: string
    age: number
    ...
  }[]
  ...
};

Some APIs only give a subset of data - in this case, only Household.people.name - which is a perfect fit for O.P.Pick. However, O.P.Pick doesn't support arrays natively, which results in types like these:

// {people: {[x: number]: {name: string}}}
type HouseholdSubset = O.P.Pick<Household, ['people', number, 'name']>;

HouseholdSubset.people isn't an array, it's still an object indexed by number - so we can't use array methods in HouseholdSubset.people, and we can't pass it to functions expecting an array. In this case, we'd like to somehow get a type of {people: {name: string}[]}.

Describe the solution you'd like

Either modify O.P.Pick or create a new type which automatically traverses arrays.

I have two solutions in mind:

  • requires TypeScript 3.7 for recursive type references, but supports infinitely nested arrays / unlimited dimensional arrays - see the implementation
  • does not require TypeScript 3.7, but only supports arrays up to a specific dimension - see the implementation for one dimensional arrays. The implementation can be expanded for additional dimensions if desired.

An example which only works using the first solution:

type FourDimensions = {
  freedom: number[][] | {
    dive: string
    pp: number
  }[][][][]
  blue: 'zenith'
}
// {freedom: number[][] | {dive: string}[][][][]}
type FreedomDive = O.P.Pick<FourDimensions, ['freedom', 'dive']>

An example which works for both solutions:

type Household = {
  id: number
  people: string[] | { // string[] added for distributive checking
    name: string
    age: number
  }[]
};
// {people: string[] | {name: string}[]}
type HouseholdSubset = O.P.Pick<Household, ['people', 'name']>

Realistically speaking, most use cases (including all of mine) will only use one dimension, so it may not worth it to require an upgrade to TypeScript 3.7 for a niche feature.

Describe alternatives you've considered

N/A

Teachability, Documentation, Adoption, Migration Strategy

If modifying O.P.Pick for this feature, this will be a breaking change for any project using O.P.Pick to pick into array properties. There should realistically not be any users affected by this, but it is a breaking change nonetheless.

If using the unlimited dimension solution, requiring TypeScript 3.7 will be a huge breaking change for most users.

Notes

I implemented this as a PR, using a different name (O.P.PickA) and using the one dimensional implementation. I realised that the details of this should be discussed in an issue before any PRs should be made - so I made this issue 馃槃

@millsp
Copy link
Owner

millsp commented Sep 30, 2019

I don't mind having breaking changes when it comes to keeping up with ts. These are advanced types, so it makes sense that versions will break. However, I would recommend not breaking the behavior of the "default" behavior. Let me explain this: I like to keep very basic features (like "Pick", "Omit") the way they are because they're "basic", working and should stay that way. They're the building blocks for all the rest of the lib.

So I'd suggest:

(And I prefer the recursive type references 馃槂)

@millsp
Copy link
Owner

millsp commented Sep 30, 2019

We could integrate these changes on the next branch for now

@millsp
Copy link
Owner

millsp commented Sep 30, 2019

I don't think you'll be able to split in such a way that it resuses PickObject since type references don't exist (yet, see microsoft/TypeScript#1213 (comment)). In this case, we should just move your version t PickArrays and the original one to PickObject.

@millsp
Copy link
Owner

millsp commented Sep 30, 2019

This is annoying if we want to bring this feature to all the types in P...

@mcpower
Copy link
Contributor Author

mcpower commented Oct 1, 2019

  • Upgrade Tuple/Pick to be able to handle that very legit use case where X[]

I don't understand - Tuple/Pick seems to be non-recursive, so I don't see how it can be "upgraded" to handle arrays.

  • Re-writing Object/P/Pick for it to receive an option array extends Boolean = True

Shouldn't the default be False to match the existing behaviour? This new behaviour should be opt-in to avoid breaking the default behaviour, if I'm understanding this correctly.

This is annoying if we want to bring this feature to all the types in P...

It may be annoying, but I think it should be doable - traversing the arrays can be relatively easily done -assuming we use recursive type references. It'll definitely be a lot of extra code, though.

If we had type references, we could probably refactor out the "array traversing" code into a separate type and share that with all types in P - but that doesn't exist yet unfortunately :(

@millsp
Copy link
Owner

millsp commented Oct 1, 2019

I don't understand - Tuple/Pick seems to be non-recursive, so I don't see how it can be "upgraded" to handle arrays.

Sorry, I meant to upgrade the non-recursive counterpart as well.

Shouldn't the default be False to match the existing behaviour? This new behaviour should be opt-in to avoid breaking the default behaviour, if I'm understanding this correctly.

Yes you're right!

@leebenson
Copy link

leebenson commented Oct 25, 2019

馃帀 eagerly awaiting the 3.7 version - thanks @mcpower!

Any way to surface this quickly without forking your OPPickArraysUnlimited branch?

My immediate use-case is drilling into arbitrarily deep GraphQL types which can be T | T[] at any level. This would be a huge help.

@mcpower
Copy link
Contributor Author

mcpower commented Oct 25, 2019

@leebenson I'll see what I can do - I've been putting this off for a while especially as 3.7 wasn't stable - but as the RC was recently released I'll probably look at this again. #60 should make this much easier - it should just be O extends object ? O extends (infer A)[] ? _Pick<A, Path, I>[] : (rest of code, starts with "OPick") : O in O.P.Pick. That is, the only difference is the addition of O extends (infer A)[] ? _Pick<A, Path, I>[] :.

I'll work on implementing this as a full-fledged PR soon (tomorrow, hopefully).

Any way to surface this quickly without forking your OPPickArraysUnlimited branch?

If the change is as easy as I think, you could probably copy the implementation of O.P.Pick, make those changes above and create a utility type in your own project - respecting ts-toolbelt's license, of course. If you need attributions on the modification, you can use "Modifications copyright (C) 2019 mcpower".

@mcpower
Copy link
Contributor Author

mcpower commented Oct 25, 2019

My immediate use-case is drilling into arbitrarily deep GraphQL types which can be T | T[] at any level. This would be a huge help.

If you're worried about 3.7 support (like I was), and that's your only use case, you can use a variant which doesn't require 3.7 - but only "dives" into 1D arrays (an example implementation should be in the original post). Let me know if you're interested in this - I can probably whip up a utility type for you.

@leebenson
Copy link

Thanks @mcpower. I have it working locally in v4.8.24, replacing a few of the internal types with publicly exported variants:

import { Any, Tuple, Iteration, Object } from "ts-toolbelt";

type _Pick<
  O extends object,
  Path extends Tuple.Tuple<Any.Index>,
  I extends Iteration.Iteration = Iteration.IterationOf<"0">
> = O extends (infer A)[] // If the object is an array
  ? A extends object //   If the array is of objects
    ? _Pick<A, Path, I>[] //     Pick from those objects
    : O //   Else, pick array for unions
  : Object.Pick<O, Path[Iteration.Pos<I>]> extends infer Picked // Else, pick the first Path
  ? {
      [K in keyof Picked]: Picked[K] extends infer Prop // Needed for the below to be distributive
        ? Prop extends object // > If it's an object
          ? Iteration.Pos<I> extends Tuple.LastIndex<Path> // & If it's the target
            ? Prop // 1-1: Pick it
            : _Pick<Prop, Path, Iteration.Next<I>> // 1-0: Continue diving
          : Prop // 0: Pick property
        : never;
    }
  : never;

/** Extract out of **`O`** the fields at **`Path`**
 * (鈿狅笍 this type is expensive)
 * @param O to extract from
 * @param Path to be followed
 * @returns **`object`**
 * @example
 * ```ts
 * ```
 */
export type Pick<O extends object, Path extends Tuple.Tuple> = _Pick<O, Path>;

Not sure if those are exact 1:1 in all cases, but it seems to be working well.

For my use-case, I'm trying to modify this to get at just the last path index, so instead of being typed as:

{
  a: {
    b: {
      c: {
        d: value
      }
    }
  }
}

It'd be typed as:

{
  d: value
}

I'm trying to drill into GraphQL Code Generator generated queries, and wind up with a type that represents an arbitrary level of the graph that can be more easily passed around as React props.

Related post for posterity: dotansimha/graphql-code-generator#2832

@leebenson
Copy link

If you're worried about 3.7 support (like I was), and that's your only use case, you can use a variant which doesn't require 3.7 - but only "dives" into 1D arrays (an example implementation should be in the original post). Let me know if you're interested in this - I can probably whip up a utility type for you.

Thanks. I'm using TS 3.7 quite extensively for this app, so happy for this to be 3.7 only, but appreciate your offer!

@mcpower
Copy link
Contributor Author

mcpower commented Oct 25, 2019

For my use-case, I'm trying to modify this to get at just the last path index [...]

Unless I'm mistaken, it seems like you want a recursive/path version of O.At instead of O.P.Path.

This is surprisingly non-trivial when it comes to nested arrays in the path... To do this with infinite nesting, we need a utility type to get the (nested) inner type of an array. However, this is impossible - see this TS Playground for an example of why it's impossible to implement.

However, if we restrict nesting to something absurd like four dimensional nested arrays, it could work, like in this TS Playground.

I think we should create a separate issue for this feature (if I'm understanding you correctly!). Judging by the linked graphql-code-generator issue, I presume you'd like to be able to do something like this:

type Nested = {
    a?: NestedA | NestedA[] | null | number;
    z: 'z';
};

type NestedA = {
    b?: NestedB | NestedB[] | null | number;
    y: 'y';
};

type NestedB = {
    c: 'c';
};

// resolves to 'c'
type C = O.P.At<Nested, ['a', 'b', 'c']>;

@stale
Copy link

stale bot commented Dec 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Dec 25, 2019
@millsp
Copy link
Owner

millsp commented Dec 30, 2019

This is not an issue anymore thanks to @mcpower

@millsp
Copy link
Owner

millsp commented Jul 21, 2020

@mcpower I am going to undo your work in the v7. You will have to use number like suggested, and it will preserve your arrays.

@mcpower
Copy link
Contributor Author

mcpower commented Jul 21, 2020

No problem. I think using number is more explicit, and avoids the extra array Boolean parameter which is nicer overall.

@thomas-at-perdoo
Copy link

I'm not sure I understand how to use this. Is there an example for P.Pick?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Add new features wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants