apollographql/apollo-link

Type error after upgrading to graphql@15

Closed this issue ยท 7 comments

I upgraded to graphql.js 15.0.0 and now I have this type error:

../../node_modules/apollo-link/lib/types.d.ts(23,56): error TS2315: Type 'ExecutionResult' is not generic.

I also got this issue while using it with NestJS:

D:\project_amanah\soluvas\soluvas-platform-nest>npm run start:dev

> soluvas-platform-nest@0.0.1 start:dev D:\project_amanah\soluvas\soluvas-platform-nest
> nest start --watch

[03:41:16] Starting compilation in watch mode...

node_modules/apollo-link/lib/types.d.ts:23:56 - error TS2315: Type 'ExecutionResult' is not generic.

23 }, C = Record<string, any>, E = Record<string, any>> = ExecutionResult<TData> & {
                                                          ~~~~~~~~~~~~~~~~~~~~~~

[03:41:32] Found 1 error. Watching for file changes.

package.json snippet:

  "dependencies": {
    "@nestjs/common": "^7.0.0",
    "@nestjs/core": "^7.0.0",
    "@nestjs/graphql": "^7.2.0",
    "@nestjs/platform-fastify": "^7.0.7",
    "@nestjs/typeorm": "^7.0.0",
    "apollo-server-fastify": "^2.11.0",
    "graphql": "^15.0.0",
    "graphql-tools": "^4.0.7",
    "mysql": "^2.18.1",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^6.5.4",
    "typeorm": "^0.2.24"
  },
  "devDependencies": {
    "@nestjs/cli": "^7.0.0",
    "@nestjs/schematics": "^7.0.0",
    "@nestjs/testing": "^7.0.0",
    "@types/express": "^4.17.3",
    "@types/jest": "25.1.4",
    "@types/node": "^13.9.1",
    "@types/supertest": "^2.0.8",
    "@typescript-eslint/eslint-plugin": "^2.23.0",
    "@typescript-eslint/parser": "^2.23.0",
    "eslint": "^6.8.0",
    "eslint-config-prettier": "^6.10.0",
    "eslint-plugin-import": "^2.20.1",
    "jest": "^25.1.0",
    "prettier": "^1.19.1",
    "supertest": "^4.0.2",
    "ts-jest": "25.2.1",
    "ts-loader": "^6.2.1",
    "ts-node": "^8.6.2",
    "tsconfig-paths": "^3.9.0",
    "typescript": "^3.7.4"
  },

Workaround: Downgrade to graphql 14.6.0

    "graphql": "<15.0.0",

Also seeing this. The TData generic doesn't appear to be customized anywhere, so I think it can be removed since the default value is the same as the new non-generic type in the graphql lib? Will open a quick PR in case people with more experience in this package agree.

๐Ÿ‘ @zackdotcomputer . everyone installing apollo and graphql recently or upgrading the graphql will definitely get this error out of nowhere.

#1263 this one doesn't break the current behavior of apollo-link

#1263 also has the advantage of doing all the package.json changes!

I do think that the end goal should be to remove this generic parameter from ExecutionResult. As the upstream comment on this change says, "it's just a glorified typecast" and increases the risk that an end user will introduce bugs by assuming the class will perform some sort of validation on the value being returned.

For now, though, I agree that #1263 does a better job of fixing this issue while minimizing downstream disruption in the short-term. I'd just suggest we should retire this typecast to bring this library in line with graphql the next time we're willing to release a breaking API change.

sgoll commented

For what it's worth, the generic parameter from ExecutionResult is (was) also used

  • via FetchResult<TData> in apollo-client's MutationUpdaterFn<T>
  • via MutationUpdaterFn<T> in @apollo/react's BaseMutationOptions<TData>
  • via BaseMutationOptions<TData> in @apollo/react-hooks's MutationHookOptions<TData>
  • via MutationHookOptions<TData> in code generated by dotansimha/graphql-code-generator

For our project, this effectively renders type-checking of the result from mutation hooks defunct since data now always is { [key: string]: any } instead of the actual type derived via code generation from the underlying schema/typedefs.

I am not sure whether this is a serious regression caused by graphql/graphql-js version 15 or is something that needs to be worked around now by higher-order libraries such as apollographql/apollo-link or dotansimha/graphql-code-generator.

#1263 addresses this and has been merged + released. Thanks all!