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

WIP: type-level assertion operator (!) #17948

Conversation

KiaraGrouwstra
Copy link
Contributor

@KiaraGrouwstra KiaraGrouwstra commented Aug 21, 2017

Fixes #14366.

I'm just extending the existing JSDocNullableType so it's usable outside of JSDoc and gets the functionality of the expression-level non-nullable / assertion operator (!).
This still evaluates too eagerly to deal with generics though.

@msftclas
Copy link

@tycho01,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@KiaraGrouwstra KiaraGrouwstra force-pushed the 14366-type-level-assertion-operator branch from dc4e63f to 7312b32 Compare August 25, 2017 14:45
@KiaraGrouwstra
Copy link
Contributor Author

@ahejlsberg: I feel a bit stuck on evaluation eagerness here -- my type Assert<T> = T!; just eagerly scrubs off the T before the type gets applied to some T value that might have | null.
Could you give a quick hint as to how to get it to evaluate when T is available? That'd help me a lot for #17884 as well.

@gcnew
Copy link
Contributor

gcnew commented Sep 1, 2017

@tycho01 While writing type class dictionaries by hand, I got stuck on #14366 as well. So I gave it a shot and got some mileage. Basic functionality is working, but higher order relations, comparability and type inference are missing. I also broke API compatibility, but it's an easy fix.

Basically, to make an operator lazy you have to create a new node for it and encode all relations.

https://github.com/gcnew/TypeScript/tree/nonNullType

@KiaraGrouwstra
Copy link
Contributor Author

@gcnew: whoa, pretty impressive progress there! I'd initially tried gone the way of adding a new node as well, but after realizing it overlapped with the JSDoc one figured that'd do to give a minimum repro of my issue with generics.

I was sorta struggling with elegantly tackling the parser part too -- looks like you handled that fairly well.
I guess my main reason to open PRs before finishing was to be able to communicate on a way forward anyway -- that part worked at least. Once you do have a PR up I guess I'll close this one.

I should try and incorporate the insights from your branch for the spread in tuple types then. Currently also still struggling with type-level calls though...

@gcnew
Copy link
Contributor

gcnew commented Sep 3, 2017

Sorry, I used bad terminology there. My addition of NonNullTypeNode was not strictly required, but I thought it'd be cleaner.

To make something lazy, you have to build an expression, instead of eagerly evaluating the said thing. For example, for keyofs that's the IndexType type; for type level indexing - IndexedAccessType. So I added a new NonNullType type. Values of these types are created by functions like getNonNullType which check whether their argument is still generic (isGenericNonNullType). If it is, they return a "to be evaluated expression", e.g. a NonNullType or else they carry the operation out and return the evaluated result.

@KiaraGrouwstra
Copy link
Contributor Author

@gcnew: yeah, I found the code in your branch pretty useful there. Much of it already looked familiar from just searching / plagiarizing existing nodes/types, but I found especially your tests to be a terrific checklist of the different scenarios. :)

@mhegazy
Copy link
Contributor

mhegazy commented Sep 7, 2017

We have discussed this in a few design meetings now, and do not feel that adding a new type operator is the right way to go.

Type operators come with a maintainability cost, as well as learning cost by adding a new concept to the language.

An alternative here is to always remove null|undefined in type positions. this allows the use case in #14366 to follow as expected.

@KiaraGrouwstra
Copy link
Contributor Author

KiaraGrouwstra commented Sep 9, 2017

@gcnew: just pushed a few examples of type subtraction including this at 4d14c9f.

@gcnew
Copy link
Contributor

gcnew commented Sep 10, 2017

@tycho01 Nice! I'm very glad you are making steady progress!

@KiaraGrouwstra
Copy link
Contributor Author

@gcnew: Thanks! You definitely helped me there! 😅

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants