-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[data] Enable manually specifying curried selector signatures #41578
[data] Enable manually specifying curried selector signatures #41578
Conversation
…tom curry signature for selectors when TypeScript inference falls short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed I'm growing more and more opposed to the work we've put in here in @wordpress/data
typings, even the work I've personally done. This adds more onto the complexity of those types in a way that I'm nervous about maintaining, but given this mostly touches some existing cruft go ahead.
interface GetEntityRecords_AsContainer extends SelectorWithCustomCurrySignature {
<K extends EntityConfig['kind']>(
state: any,
kind: K,
key: Extract<EntityConfig, { kind: K }>['keyType']
): Extract<EntityConfig, { kind: K }>['record'];
CurriedSignature: <K extends EntityConfig['kind']>(
kind: K,
key: Extract<EntityConfig, { kind: K }>['keyType']
) => Extract<EntityConfig, { kind: K }>['record'];
} The repetition here seems perfect for a type constructor that would just thake the |
@sarayourfriend I couldn't get that to work, unfortunately :-( The moment we separate the parameters from the signature, we're toast: TS Playground |
Agreed, I am also growing more and more skeptical of the approach we initially took. This PR is for your branch, not for trunk so I'll go ahead and merge it. That being said, I'd rather regroup and start shipping incremental improvements such as support for |
…hrough signature currying in `@wordpress/data`. Declare GetEntityRecord as a *callable interface* that is callable as usually, but also ships another signature without the state argument. This works around a TypeScript limitation that doesn't allow currying generic functions: ```ts type CurriedState = F extends ( state: any, ...args: infer P ) => infer R ? ( ...args: P ) => R : F; type Selector = <K extends string | number>( state: any, kind: K, key: K extends string ? 'string value' : false ) => K; type BadlyInferredSignature = CurriedState< Selector > // BadlyInferredSignature evaluates to: // (kind: string number, key: false | "string value") => string number ``` The signature without the state parameter shipped as CurriedSignature is used in the return value of `select( coreStore )`. See #41578 for more details. This commit includes a docgen update to add support for typecasting selectors
…tEntityRecords through currying (#44453) Declare GetEntityRecord as a *callable interface* that is callable as usually, but also ships another signature without the state argument. This works around a TypeScript limitation that doesn't allow currying generic functions: ```ts type CurriedState = F extends ( state: any, ...args: infer P ) => infer R ? ( ...args: P ) => R : F; type Selector = <K extends string | number>( state: any, kind: K, key: K extends string ? 'string value' : false ) => K; type BadlyInferredSignature = CurriedState< Selector > // BadlyInferredSignature evaluates to: // (kind: string number, key: false | "string value") => string number ``` The signature without the state parameter shipped as CurriedSignature is used in the return value of `select( coreStore )`. See #41578 for more details. This commit includes a docgen update to add support for typecasting selectors
…tEntityRecords through currying (#44453) Declare GetEntityRecord as a *callable interface* that is callable as usually, but also ships another signature without the state argument. This works around a TypeScript limitation that doesn't allow currying generic functions: ```ts type CurriedState = F extends ( state: any, ...args: infer P ) => infer R ? ( ...args: P ) => R : F; type Selector = <K extends string | number>( state: any, kind: K, key: K extends string ? 'string value' : false ) => K; type BadlyInferredSignature = CurriedState< Selector > // BadlyInferredSignature evaluates to: // (kind: string number, key: false | "string value") => string number ``` The signature without the state parameter shipped as CurriedSignature is used in the return value of `select( coreStore )`. See #41578 for more details. This commit includes a docgen update to add support for typecasting selectors
…tEntityRecords through currying (#44453) Declare GetEntityRecord as a *callable interface* that is callable as usually, but also ships another signature without the state argument. This works around a TypeScript limitation that doesn't allow currying generic functions: ```ts type CurriedState = F extends ( state: any, ...args: infer P ) => infer R ? ( ...args: P ) => R : F; type Selector = <K extends string | number>( state: any, kind: K, key: K extends string ? 'string value' : false ) => K; type BadlyInferredSignature = CurriedState< Selector > // BadlyInferredSignature evaluates to: // (kind: string number, key: false | "string value") => string number ``` The signature without the state parameter shipped as CurriedSignature is used in the return value of `select( coreStore )`. See #41578 for more details. This commit includes a docgen update to add support for typecasting selectors
What problem is this PR solving?
The
CurriedState
provided in #39081 does not correctly curry complex type signatures.What's
CurriedState
?The
CurriedState
type is provided to remove thestate
argument from selectors function signatures:It's needed to transform the definitions in
selectors.ts
that accept thestate
argument, to signatures exposed byuseSelect(select => select( store )./*...*/)
that do not accept thestate
argument`Where does it fall short?
Here's a minimalistic example:
What solution does this PR propose?
I don't believe TypeScript supports the kind of automatic transformation the
CurriedState
wants to do.This PR provides an escape hatch to manually specify the curried signatures when TypeScript falls short:
What else has been tried?
I've tried a ton of different ways to infer things automatically.
Here's the basic setup used in all the attempts:
Instead of trying to do the
Curry
, which clones the signature with one argument less, I tried to just clone the signature which seems easier and requires less complexity.My ideal
Clone
type helper would do the following:The only constraint is I am not allowed to simply do this:
After many hours, I don't think such type can be created in a way that works with generic functions.
Universal clone like
CloneAnyFunction<F>
In this scenario, we need to derive F somehow
One way to infer the function type is to use the infer TypeScript feature:
InferredSignature type is not constrained in the same way as the actual arguments are, so this is dead end.
Another way would be to use the typeof operator:
It seems like a step in the right direction – the TypeOfSignature type reflects all the constraints we care about.
Unfortunately, there isn't much we can do with them. For example, we can't extract them with the Parameters<> helper:
SelectorArguments is the same as args of the InferredSignature – which tells me that raw
typeof
is a dead end.There's a twist on
typeof
called "instantiation expressions":It is better in that the signature reflects the actual constraints imposed on the getEntityRecords definition. The problem now is that it's only for the specific kind value of 'root'. We want to make it work for any possible kind value.
Let's try passing a type union:
Nope, this signature lacks the constraints as well.
Finally, we could try parametrizing the signature type as follows:
The problem here is that we'd need to specify the exact same constraints as getEntityRecords, otherwise we get this error:
The only solution seems to be really:
The only thing we did here, though, is we moved the Kind from a function to a proxy type. We still have all the same problems as before, e.g.:
This tells me that TypeScript doesn't support a universal Clone type such as the one below:
F must be either instantiated type or parametrized with generics, but there aren't any TS features that enable extracting the nuanced, constrained argument types from it.
Parametrized clone like
CloneAnyFunction<Arg0, Arg1, Return>
I've also tried my luck with Parametrization
There aren't many ways of formulating such a parametrized clone type I can think of. I only found the following Curry implementation on StackOverflow:
Unfortunately, we're running into the same kind of "insufficient constraints" errors as before:
We could, of course, specify the constraints explicitly:
But even after all this effort, we're back to the non-specific type signature.
Parametrized arguments tuple
Once the type constraints are baked into a function type signature, they are stuck there. There seems to be no TypeScript feature that can split the function apart into a constrained arguments type and a constrained return type.
But what if we define the arguments tuple manually?
Oki, now let's slice it:
Uh-oh! selectorWithSlicedArgs accepts the unconstrained arguments. We can't even slice the parametrized tuple in a way that preserves the constraints! This leaves us with only one option: provide the curried and uncurried type signatures manually :(
And it's exactly what this PR proposes.
Manually specifying both signatures
We want to do two things with selectors like
getEntityRecords
:Let's define a type that is both callable and also contains information about the manually-defined curried signature:
Here's a proof it can be called just like a regular function:
It even respects our type constraints, great!
Now, let's try currying:
Brilliant! And, in case you wondered,
CurriedState
can still automatically curry any other function:Testing Instructions
The checks are all red, that's fine – it's a PR to another branch based on a pretty old trunk
cc @dmsnell @sarayourfriend @sirreal @jsnajdr