-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore(gatsby): migrate derived-types to TypeScript #22442
Conversation
Sorry for the late response on this. Can you please rebase onto master? Thanks! |
da5b9e0
to
85eecd6
Compare
typeComposer, | ||
}: { | ||
typeComposer: AllTypeComposer | ||
}): any => typeComposer.getExtension(`derivedTypes`) || new Set() |
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.
I'm looking into the return type here.
I can see in https://graphql-compose.github.io/docs/api/ObjectTypeComposer.html that the returned type is Extensions
. Unfortunately that does not describe the type. The default being a Set
is a hint, but a Set
of what ...
https://github.com/graphql-compose/graphql-compose/blob/master/src/utils/definitions.d.ts#L16 implies that it is:
export type Extensions = {
[key: string]: any;
directives?: ExtensionsDirective[];
};
But that doesn't seem right since we default to a Set
, AND we call .add
on it in addDerivedType()
(see later in this file).
const derivedTypes = getDerivedTypes({ typeComposer })
typeComposer.setExtension(`derivedTypes`, derivedTypes.add(derivedTypeName))
So this feels a little troubling, to say the least.
@vladar do you perhaps have an insight here? Do we have a bug in our code or am I looking at the wrong docs?
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.
I can see in https://graphql-compose.github.io/docs/api/ObjectTypeComposer.html that the returned type is
Extensions
It is the returned type of getExtensions
method. But we are looking at getExtension
which has any
as the returned type. Extension can be basically anything we want. In our case derivedTypes
extension is a Set | void
.
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.
Thanks for sharing your insight. I tried to fix it.
55ed26f
to
482d384
Compare
482d384
to
75b40c7
Compare
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.
Great! Thank you 💜
Description
Part of #21995
I migrated
derived-types.js
to TS.Documentation
Related Issues
#21995