apollographql / apollo-link

:link: Interface for fetching and modifying control flow of GraphQL requests
https://www.apollographql.com/docs/link/
MIT License
1.44k stars 344 forks source link

Type error after upgrading to graphql@15 #1261

Closed tyler-johnson closed 4 years ago

tyler-johnson commented 4 years ago

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.
ceefour commented 4 years ago

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",
zackdotcomputer commented 4 years ago

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.

ceefour commented 4 years ago

:+1: @zackdotcomputer . everyone installing apollo and graphql recently or upgrading the graphql will definitely get this error out of nowhere.

kamilkisiela commented 4 years ago

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

zackdotcomputer commented 4 years ago

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 4 years ago

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

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.

hwillson commented 4 years ago

https://github.com/apollographql/apollo-link/pull/1263 addresses this and has been merged + released. Thanks all!