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

typesafe-joi blows up TypeScript 3.2+ #28873

Closed
hjkcai opened this issue Dec 6, 2018 · 9 comments · Fixed by #29179
Closed

typesafe-joi blows up TypeScript 3.2+ #28873

hjkcai opened this issue Dec 6, 2018 · 9 comments · Fixed by #29179
Assignees
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output

Comments

@hjkcai
Copy link

hjkcai commented Dec 6, 2018

TypeScript Version: 3.3.0-dev.20181205

Search Terms:

Maximum call stack size exceeded

Code Repo:

https://github.com/hjkcai/typesafe-joi

Reproduction:

mkdir typesafe-joi-issue && cd typesafe-joi-issue
yarn add typescript@next @types/node typesafe-joi joi
echo '{"compilerOptions":{"strict":true}}' > tsconfig.json
echo 'import * as Joi from "typesafe-joi"; console.log(Joi)' > index.ts
npx tsc -p .

Expected behavior:

tsc should work.

I admit that typesafe-joi uses lots of type recursion, but it works fine in TypeScript 3.1. After upgrading to TypeScript 3.2, the error comes.

Actual behavior:

tsc itself encountered 'Maximum call stack size exceeded' (No stack trace printed)

@j-oliveras
Copy link
Contributor

Without npx using command node_modules\.bin\tsc -p . I see the callstack:

RangeError: Maximum call stack size exceeded
    at resolveNameHelper (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:26036:35)
    at resolveName (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:26034:20)
    at resolveEntityName (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:26728:26)
    at resolveTypeReferenceName (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31885:20)
    at getTypeFromTypeReference (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:32061:30)
    at getTypeFromTypeNode (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:33421:28)
    at getConstraintFromTypeParameter (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31747:72)
    at fillMissingTypeArguments (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31337:49)
    at getTypeFromClassOrInterfaceReference (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31837:78)
    at getTypeReferenceTypeWorker (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31950:24)

Running node setting a stack-trace-limit to 100000 (to see the full stack trace) with node --stack-trace-limit=100000 node_modules\typescript\bin\tsc -p .: full stack trace (is to large to paste here).

Seems that the recursion starts at call to getTypeFromTypeReference from checkTypeReferenceNode and the repetition is this callstack fragment:

    at getTypeFromTypeNode (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:33422:28)
    at getConstraintFromTypeParameter (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31748:72)
    at fillMissingTypeArguments (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31338:49)
    at getTypeFromClassOrInterfaceReference (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31838:78)
    at getTypeReferenceTypeWorker (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31951:24)
    at getTypeReferenceType (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:31893:24)
    at getTypeFromTypeReference (Z:\tmp\ts\node_modules\typescript\lib\tsc.js:32063:28)

@weswigham weswigham added Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output labels Dec 6, 2018
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.3 milestone Dec 8, 2018
@ahejlsberg
Copy link
Member

The issue here is that AbstractSchema specifies itself as a constraint and relies on type argument defaults in that specification:

interface AbstractSchema<Schema extends AbstractSchema = any, Value = any> extends JoiObject { ... }

The use of default type arguments in combination with the recursive constraint causes an infinite loop. An easy fix for now is to specify the type arguments:

interface AbstractSchema<Schema extends AbstractSchema<any, any> = any, Value = any> extends JoiObject { ... }

@ahejlsberg
Copy link
Member

@weswigham Best I can tell the infinite loop is caused by fillMissingTypeArguments attempting to get the constraint of the type parameter, which leads back to the same type, which leads back to needing to add default type arguments, and down the rabbit hole we go. I think you were last person to work on this so I'm assigning it over to you.

@ahejlsberg ahejlsberg assigned weswigham and unassigned ahejlsberg Dec 9, 2018
@hjkcai
Copy link
Author

hjkcai commented Dec 10, 2018

@ahejlsberg I assume TypeScript will fill in the type arguments automatically, because AbstractSchema have default type arguments. I will change my code later. Thanks a lot!

@EvgenyMuryshkin
Copy link

I made local fix, you can try to apply and see if it helps
EvgenyMuryshkin@9f88c2f

@ahejlsberg
Copy link
Member

@EvgenyMuryshkin Doh! You're right, it definitely should be calling getConstraintOfTypeParameter (which checks for circularities) and not getConstraintFromTypeParameter (which is a worker function that doesn't). Thanks for catching this, I will turn it into a PR.

@ahejlsberg
Copy link
Member

@weswigham @rbuckton @DanielRosenwasser I'm thinking that all of the proposed solutions to this issue paper over the the real culprit: Type parameter defaults should only be allowed to reference previously declared type parameters (i.e. type parameters declared to the left). I alluded to this in my comment here. Such a rule would be analogous to JavaScript's restriction that parameter defaults can only reference parameters to the left, and it would make nonsensical constructs such as the following become errors:

type Foo<T = T> = T[];
type Bar<T = U, U = T> = [T, U];

After all, when a type parameter default comes into play, we know that type arguments for that type parameter and type parameters to the right of it weren't specified, so there is nothing meaningful to reference. The OP issue is caused by us trying to come up with a meaningful default for a forward reference (and our answer effectively amounts to the constraint of the forwardly referenced type parameter), but I'm thinking it should just be an error (and we should just use the error type).

I suppose you could make a weak argument that type parameter defaults in generic functions should be able to reference later type parameters because type inference might produce useful inferred types, but I have yet to see a real world example where that is useful.

I have a branch that implements this restriction and I'm the process of running validating all of Definitely Typed on it. So far nothing seems to be affected.

@EvgenyMuryshkin
Copy link

We are using this project and I had to tweak type definition a bit after 3.2 migration
Seems that there is a circular reference in types

https://github.com/projectstorm/react-diagrams/blob/master/src/BaseEntity.ts

@hjkcai
Copy link
Author

hjkcai commented Apr 7, 2019

Hey guys, typesafe-joi make typescript crash again in #30794. Please help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Crash For flagging bugs which are compiler or service crashes or unclean exits, rather than bad output
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants