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

Exact type assertions #21

Closed
wants to merge 2 commits into from

Conversation

BendingBender
Copy link
Collaborator

@BendingBender BendingBender commented Mar 25, 2019

This PR implements exact type assertions as discussed here.

Waiting for #20.

@BendingBender BendingBender changed the title Exact type checks Exact type assertions Mar 25, 2019
@BendingBender BendingBender marked this pull request as ready for review March 29, 2019 07:12
@SamVerschueren
Copy link
Collaborator

Hi @BendingBender. We should take a look if we can get this one merged!

Before I dive into the details, could you explain what your changes actually do? It will probably help me in understanding what changed and what exactly is going on.

Thanks for this PR btw, it's open for too long :).

@SamVerschueren
Copy link
Collaborator

This PR is blocked right now.

The reason for this is that it uses string matching to assert the types. This means that string | number is not the same as number | string. This is a problem in dtslint as well microsoft/dtslint#191.

We have to find a way to do this on type system level or something so the order doesn't matter and users like it because a minor change in the type definitions doesn't cause a headache.

If people have ideas on how to fix this, feel free to chime in.

@orta Do you have an idea how we would be able to do this? Some TS API or something that could help us out with this?

@orta
Copy link
Contributor

orta commented Sep 19, 2019

I'm not sure how much of this is public api, but you can get a type checker from the program ( program.getTypeChecker() ) which then has ways to get access to the types (checker.getTypeOfSymbolAtLocation for example)

Then you're kinda at a dead end, see: microsoft/TypeScript#9879 microsoft/TypeScript#9943

That said, it looks like https://www.npmjs.com/package/ts-simple-type looks like it can handle most cases

@SamVerschueren
Copy link
Collaborator

That's a good pointer in a direction I like. Played with it a little but it's pretty hard due to the fact that the TS compiler isn't really co-operating 😂.

For example

expectType<string>(concat('foo', 'bar'));

I want to infer the return type of the concat() call, but it looks like I tried everything without luck.

Let's asume that node is the expectType node, then the following call fails with

typeChecker.getResolvedSignature(node.expression.arguments[0]);
//=> 'Debug Failure. False expression.'

typeChecker.getResolvedSignature(node.expression.arguments[0].expression);
//=> 'Debug Failure. Branch in \'resolveSignature\' should be unreachable. SyntaxKind: Identifier'

The getResolvedSignature expects a CallExpression where arguments[0] is IMO a CallExpression. I might be missing something but what am I doing wrong here 😂? Any help would be appreciated.

@SamVerschueren
Copy link
Collaborator

I got things working 🎉

I can now detect that the type is too wide. But because I don't use string matching string | number is the same as number | string. Or even more complex types. string | Promise<boolean | number> === Promise<number | boolean> | string.

Still have to fix some things though, but this approach already looks promising.

@SamVerschueren
Copy link
Collaborator

It works for almost everything, except when I wanted to add tests for RxJS, it fails. I created an issue in ts-simple-type for that which you can find here runem/ts-simple-type#54.

We really need that type relation API...

@SamVerschueren
Copy link
Collaborator

Everything works locally. Tests are added. I only have to wrap it up tomorrow and then this is done.

I removed ts-simple-type because it failed in some use cases, mainly generica. I wanted the real deal, being the TypeScript type checker. So what did I do? I copied over the TypeScript package, exposed one extra method (isAssignableTo) and live happily ever after. Everything works out of the box.

I know, this is not an ideal situation, but it was the only way I could make it work like I wanted it to work... I hope the TypeScript team sees the benefits in exposing that API publically so I don't have to workaround the problem...

So this functionality is coming to you in this weekend.

@SamVerschueren
Copy link
Collaborator

SamVerschueren commented Sep 26, 2019

Implemented in e4c78c7

@BendingBender BendingBender deleted the exact-type-checks branch June 3, 2021 18:49
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.

3 participants