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

Sync type TS definitions with Flow #2109

Merged
merged 7 commits into from Aug 23, 2019

Conversation

JacksonKearl
Copy link

Specifically:
Add extensions field in lots of places
Reorder fields

Both this and #2106 add Path.d.ts, but they're the same. If theres a merge conflict, just pick one.

Specifically:
Add `extensions` field in lots of places
Reorder fields

Both this and graphql#2106 add Path.d.ts, but they're the same. If theres a merge conflict, just pick one.
@JacksonKearl
Copy link
Author

JacksonKearl commented Aug 23, 2019

This will need to wait until #2106 is in, then we can rebase off of it. Currently ResponsePath is undefined.

@@ -8,4 +8,4 @@ export const GraphQLID: GraphQLScalarType;

export const specifiedScalarTypes: ReadonlyArray<GraphQLScalarType>;

export function isSpecifiedScalarType(type: GraphQLScalarType): boolean;
export function isSpecifiedScalarType(type: unknown): type is GraphQLScalarType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought unknown available only in TS 3.0?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to any

@@ -40,6 +30,8 @@ export const SchemaMetaFieldDef: GraphQLField<any, any>;
export const TypeMetaFieldDef: GraphQLField<any, any>;
export const TypeNameMetaFieldDef: GraphQLField<any, any>;

export const introspectionTypes: ReadonlyArray<any>;
export const introspectionTypes: ReadonlyArray<
GraphQLObjectType | GraphQLEnumType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to make it just GraphQLType since we can latter add different types.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

@@ -285,13 +288,16 @@ export class GraphQLScalarType {
serialize: GraphQLScalarSerializer<any>;
parseValue: GraphQLScalarValueParser<any>;
parseLiteral: GraphQLScalarLiteralParser<any>;
extensions: Maybe<Record<string, any>>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make them ReadOnly both in class definitions and inside configs?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

@JacksonKearl
Copy link
Author

@IvanGoncharov should be good to go now

@IvanGoncharov IvanGoncharov merged commit a482d3b into graphql:master Aug 23, 2019
@IvanGoncharov
Copy link
Member

@JacksonKearl Merged 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bug fix 🐞 requires increase of "patch" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants