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

fix: Permit isDeprecated field in presence of deprecationReason #2684

Conversation

trevor-scheer
Copy link
Contributor

Currently, the typings for a GraphQLField and GraphQLEnumValue are at odds with their runtime expectations.

The typings require the existence of isDeprecated.

isDeprecated: boolean,

isDeprecated: boolean,

However, at runtime, we throw an error if isDeprecated exists.

devAssert(
!('isDeprecated' in fieldConfig),
`${config.name}.${fieldName} should provide "deprecationReason" instead of "isDeprecated".`,
);

devAssert(
!('isDeprecated' in valueConfig),
`${typeName}.${valueName} should provide "deprecationReason" instead of "isDeprecated".`,
);

This PR proposes that we permit the presence of that field as long as deprecationReason is also provided. Alternatively, we could update the types and make isDeprecated optional.

I'm currently running into this error in a PR to the apollo-tooling repo to migrate to v15:
apollographql/apollo-tooling#1743

@IvanGoncharov
Copy link
Member

export type GraphQLEnumValueConfig /* <T> */ = {|
description?: ?string,
value?: any /* T */,
deprecationReason?: ?string,
extensions?: ?ReadOnlyObjMapLike<mixed>,
astNode?: ?EnumValueDefinitionNode,
|};
export type GraphQLEnumValue /* <T> */ = {|
name: string,
description: ?string,
value: any /* T */,
isDeprecated: boolean,
deprecationReason: ?string,
extensions: ?ReadOnlyObjMap<mixed>,
astNode: ?EnumValueDefinitionNode,
|};

@trevor-scheer For creating types you should use only GraphQ*Config types and they deliberately missing isDeprecated. We plan to remove isDeprecated completely in v16 (except of introspection) so removing it from config types is part of our deprecation plan and was done in 15.0.0 as a breaking change.

@trevor-scheer
Copy link
Contributor Author

For anyone else that may run into this issue, here is my commit which resolved the issue. Exactly as @IvanGoncharov explained. Thank you!

e9db99d (#1743)

@trevor-scheer trevor-scheer deleted the permit-is-deprecated-with-deprecationReason branch July 2, 2020 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants