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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add the TypeOperatorType that represents an op result on a type ("keyof") #614

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

shlomiassaf
Copy link
Contributor

@shlomiassaf shlomiassaf commented Oct 10, 2017

The TypeOperatorType represent the typescript TypeOperator node type.

class A {
  a: string;
  b: number;
}

class B<T extends keyof A> {}
class B<T extends A> {}

In the example above, the Reflection instances for class B and class C will include a TypeParameterType T that has a constraint.

For class C it will be a TypeReferenceType (reference to reflection of A)

For class B it will be a TypeOperatorType with a target property holding a TypeReferenceType instance (reference to reflection of A)

Before this PR the behaviour is to include a union type that contains IntrinsicType of string's that represent the properties of class A ('a', 'b');

While this is logically true it is not accurate, the actual type is the list of keys of another type.
The documentation should show the connection between T and A and not just show a list of strings.

Some classes or types might have 10-15 or more members, it does not scale

@shlomiassaf
Copy link
Contributor Author

TODO: Add serialization support to #597 or here...

@shlomiassaf
Copy link
Contributor Author

image

image

@aciccarello
Copy link
Collaborator

@shlomiassaf Great work implementing this! Can you explain a little more your motivation for the name TypeOperator? Is that the TypeScript node name? If it is that makes sense, but my initial reaction was that since keyof is the only operator we should just refer to it as keyof.

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Oct 10, 2017

@aciccarello
Yes, it is the TypeScript name, see this line at the end.

It reference to the Node for that type (i.e. TypeNode) hence the name TypeOperatorNode
The SyntaxKind for this node (i.e. node.kind) is TypeOperator and this is where the name comes from, consistent with other type names in the project.

If you look at the source code you will see that the TypeOperatorNode has a operator property, That property is fixed to SyntaxKind.KeyOfKeyword.

So, property is fixed to keyof and indeed keyof is the only operator there is (for types).

However, if think about it, there is no reason for the TS team to create a fixed operator property for this node if keyof is the only operator...
It must be a place holder for future types, when and if they come.

So, to be prepared I followed their structure and it means that if they add more operators we are ready for it without breaking anything.

@aciccarello aciccarello merged commit b328cac into TypeStrong:master Oct 11, 2017
@shlomiassaf shlomiassaf deleted the keyof-type branch October 15, 2017 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants