-
Notifications
You must be signed in to change notification settings - Fork 198
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
feat: Improved Promise handling to support packages like Prisma #1924
Conversation
@@ -27149,6 +27149,8 @@ | |||
}, | |||
"SingleDefUnitChannel": { | |||
"enum": [ | |||
"text", |
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.
Somehow this type started getting out of order and breaking CI:
I changed its order so tests can pass, I'm also almost sure enum
is order insensitive.
Hey @domoritz, it's ready for review :) |
I was waiting for tests to pass. I'll check when I'm back from traveling. |
me too 😭 |
@domoritz done! Codecov action failed but all missing coverage entries are from safety checks that should never hit in the current typescript version. |
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.
This is great. Just a few comments.
if (typeSymbol.name === "Promise" || typeSymbol.name === "PromiseLike") { | ||
// Promise without type resolves to Promise<any> | ||
if (!node.typeArguments || node.typeArguments.length === 0) { | ||
return new AnyType(); |
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.
Can we move this logic into the PromiseNodeParser?
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.
Actually not, It might be a skill issue of mine but I couldn't get it to work without this statement.
When I remove it, this is the error thrown:
● valid-data-type › promise-extensions
TypeError: Cannot read properties of undefined (reading 'getId')
123 |
124 | // Check for simple type equality
> 125 | if (source.getId() === target.getId()) {
| ^
126 | return true;
127 | }
128 |
at getId (src/Utils/isAssignableTo.ts:125:16)
at src/NodeParser/ConditionalTypeNodeParser.ts:44:77
at predicate (src/Utils/narrowType.ts:59:12)
at ConditionalTypeNodeParser.createType (src/NodeParser/ConditionalTypeNodeParser.ts:44:41)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at TypeAliasNodeParser.createType (src/NodeParser/TypeAliasNodeParser.ts:40:43)
at AnnotatedNodeParser.createType (src/NodeParser/AnnotatedNodeParser.ts:34:47)
at ExposeNodeParser.createType (src/ExposeNodeParser.ts:23:45)
at CircularReferenceNodeParser.createType (src/CircularReferenceNodeParser.ts:24:43)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at TypeReferenceNodeParser.createType (src/NodeParser/TypeReferenceNodeParser.ts:78:37)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at PromiseNodeParser.createType (src/NodeParser/PromiseNodeParser.ts:71:47)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at TypeReferenceNodeParser.createType (src/NodeParser/TypeReferenceNodeParser.ts:78:37)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at createType (src/NodeParser/UnionNodeParser.ts:22:45)
at Array.map (<anonymous>)
at UnionNodeParser.map [as createType] (src/NodeParser/UnionNodeParser.ts:21:14)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at TypeAliasNodeParser.createType (src/NodeParser/TypeAliasNodeParser.ts:40:43)
at AnnotatedNodeParser.createType (src/NodeParser/AnnotatedNodeParser.ts:34:47)
at ExposeNodeParser.createType (src/ExposeNodeParser.ts:23:45)
at CircularReferenceNodeParser.createType (src/CircularReferenceNodeParser.ts:24:43)
at ChainNodeParser.createType (src/ChainNodeParser.ts:35:54)
at TopRefNodeParser.createType (src/TopRefNodeParser.ts:14:47)
at createType (src/SchemaGenerator.ts:30:36)
at Array.map (<anonymous>)
at SchemaGenerator.map [as createSchemaFromNodes] (src/SchemaGenerator.ts:29:37)
at SchemaGenerator.createSchemaFromNodes [as createSchema] (src/SchemaGenerator.ts:25:21)
at Object.createSchema (test/utils.ts:61:34)
Co-authored-by: Dominik Moritz <domoritz@gmail.com>
is there anything left? |
closes #1923
closes kitajs/kitajs#280
This PR adds support to any
Promise<T> | PromiseLike<T>
type to be extracted intoT
. As promises cannot be transformed into any value and can only be used once resolved, this is a safe assumption to extract.My test case at test/valid-data/promise-extensions/main.ts ensures every "thenable" type is correctly extracted.
All of them results into the following json schema:
(with their respective type name)
My base comparison was to strictly follow what the native
Awaited
native type added by microsoft/TypeScript#45350.It works without much hassle by using a internal
ts.TypeChecker
property. I also added https://www.npmjs.com/package/ts-expose-internals as a dev dependencyVersion
Published prerelease version:
v2.2.0-next.0
Changelog
🎉 This release contains work from new contributors! 🎉
Thanks for all your work!
❤️ Sampsa Kaskela (@SampsaKaskela)
❤️ Rama Krishna Ghanta (@ramaghanta)
❤️ Tim (@timothympace)
🚀 Enhancement
🐛 Bug Fix
🔩 Dependency Updates
Authors: 6