chore(api-graphql): Bump graphql to 14.5.0 #8984
Conversation
wlee221
changed the title
|
Getting these errors on Errors``` @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (207,20): Property 'operation' does not exist on type 'DefinitionNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } @aws-amplify/api-graphql: { @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (297,17): Argument of type 'string | DocumentNode' is not assignable to parameter of type 'ASTNode'.\n" + @aws-amplify/api-graphql: " Type 'string' is not assignable to type 'ASTNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } @aws-amplify/api-graphql: { @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (392,18): Argument of type 'string | DocumentNode' is not assignable to parameter of type 'ASTNode'.\n" + @aws-amplify/api-graphql: " Type 'string' is not assignable to type 'ASTNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } @aws-amplify/api-graphql: { @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (207,20): Property 'operation' does not exist on type 'DefinitionNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } @aws-amplify/api-graphql: { @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (297,17): Argument of type 'string | DocumentNode' is not assignable to parameter of type 'ASTNode'.\n" + @aws-amplify/api-graphql: " Type 'string' is not assignable to type 'ASTNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } @aws-amplify/api-graphql: { @aws-amplify/api-graphql: message: "/Users/willeea/Documents/amplify/amplify-js/packages/api-graphql/src/GraphQLAPI.ts (392,18): Argument of type 'string | DocumentNode' is not assignable to parameter of type 'ASTNode'.\n" + @aws-amplify/api-graphql: " Type 'string' is not assignable to type 'ASTNode'.", @aws-amplify/api-graphql: level: 'error' @aws-amplify/api-graphql: } ``` |
| @@ -294,7 +295,7 @@ export class GraphQLAPIClass { | |||
| }; | |||
|
|
|||
| const body = { | |||
| query: print(query), | |||
| query: print(query as DocumentNode), | |||
There was a problem hiding this comment.
This is to address "Type 'string' is not assignable to type 'ASTNode'." error.
I'm confident that this query is always of type DocumentNode, because query is always run through parsing here before _graphql function is called:
amplify-js/packages/api-graphql/src/GraphQLAPI.ts
Lines 225 to 228 in ec78195
in which parse returns DocumentNode object (source).
There was a problem hiding this comment.
Either we can do as DocumentNode here, or we can create an internal type
const ParsedGraphQLOption = Omit<GraphQLOptions, 'query'> & {query: DocumentNode}but I kept it simple for now. Opinions welcome!
| const { | ||
| definitions: [{ operation: operationType }], | ||
| } = doc; | ||
| const definitions = doc.definitions as ReadonlyArray< | ||
| OperationDefinitionNode | ||
| >; | ||
| const [{ operation: operationType }] = definitions; |
There was a problem hiding this comment.
I'm not so confident with this one. It seems that doc.definitions doesn't always contain an object with operation field. You can check out its typing here: https://github.com/graphql/graphql-js/blob/7b389be745eaeda2a4f9ca33a3db93717f21447e/tstypes/language/ast.d.ts#L186-L195
OperationDefinitionNode is the one that had it, so I casted definitions to it.
There was a problem hiding this comment.
Update: I found that @types/[email protected] has the same type, so this is not a change from 14.0.0 -> 14.5.0.
Codecov Report
@@ Coverage Diff @@
## main #8984 +/- ##
==========================================
- Coverage 78.02% 78.01% -0.01%
==========================================
Files 250 250
Lines 18109 18108 -1
Branches 3882 3882
==========================================
- Hits 14129 14127 -2
- Misses 3850 3851 +1
Partials 130 130
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
LGTM! Just checking if we can update to [email protected] and import from the top of the package.
| @@ -49,7 +49,7 @@ | |||
| "@aws-amplify/cache": "4.0.21", | |||
| "@aws-amplify/core": "4.3.1", | |||
| "@aws-amplify/pubsub": "4.1.11", | |||
| "graphql": "14.0.0", | |||
| "graphql": "14.5.0", | |||
There was a problem hiding this comment.
Why not using [email protected]?
This reverts commit f8e79e4.
|
Update: I tried bumping it to 15.6.0, but multiple datastore e2e tests were failing -- https://app.circleci.com/pipelines/github/aws-amplify/amplify-js/9109/workflows/3489ebeb-d07a-4324-b1d8-c108b7463677. I'm putting it back to 14.5.0 to prevent breaking changes, but we can revisit this later or when developers create an issue for this. I'll go ahead and merge this Monday, if there are no objections. cc @elorzafe |
|
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Description of changes
Bumps api-graphql to 14.5.0 to address #8983. The specific commit we need is graphql/graphql-js#2102, which adds strict types for graphql modules. This resolves TypeScript errors when you use 1) Angular 12+, and/or 2) strict mode on TypeScript.
Issue #, if available
Fixes #8983
aws-amplify/docs#2918, aws-amplify/docs#3140, aws-amplify/docs#3376
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.