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

Use renamed @graphql-codegen/cli instead of graphql-code-generator #60783

Closed
wants to merge 11 commits into from

Conversation

watson
Copy link
Contributor

@watson watson commented Mar 20, 2020

The dev-dependency module graphql-code-generator has been deprecated and new development continues under the name @graphql-codegen/cli. The old version is no longer maintained.

An upgrade-guide has been published for upgrading from graphql-code-generator to @graphql-codegen/cli: https://graphql-code-generator.com/docs/migration/from-0-18

@watson watson added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 20, 2020
@watson watson self-assigned this Mar 20, 2020
@watson
Copy link
Contributor Author

watson commented Mar 20, 2020

CI fails the "Check licenses" check with the following errors:

succ All production dependency licenses are allowed
ERROR Non-conforming licenses:
        @apollographql/graphql-language-service-interface
          version: 2.0.2
          all licenses: Custom: https://github.com/graphql/graphql-language-service
          invalid licenses: Custom: https://github.com/graphql/graphql-language-service
          path: node_modules/apollo-language-server/node_modules/@apollographql/graphql-language-service-interface
        @apollographql/graphql-language-service-parser
          version: 2.0.2
          all licenses: Custom: http://graphql.org/
          invalid licenses: Custom: http://graphql.org/
          path: node_modules/apollo-language-server/node_modules/@apollographql/graphql-language-service-parser
        @apollographql/graphql-language-service-types
          version: 2.0.2
          all licenses: Custom: https://flowtype.org/
          invalid licenses: Custom: https://flowtype.org/
          path: node_modules/apollo-language-server/node_modules/@apollographql/graphql-language-service-types
        @apollographql/graphql-language-service-utils
          version: 2.0.2
          all licenses: Custom: https://github.com/graphql/graphql-language-service
          invalid licenses: Custom: https://github.com/graphql/graphql-language-service
          path: node_modules/apollo-language-server/node_modules/@apollographql/graphql-language-service-utils

The license issue is also being discussed by others here: apollographql/apollo-tooling#1553

@spalger
Copy link
Contributor

spalger commented Mar 24, 2020

When dependencies have a license that can't be properly parsed we can override the setting via https://github.com/elastic/kibana/blob/e60f7d62edaacd969ec8ff0971117f3687e18617/src/dev/license_checker/config.ts#L79-L107

Looked at a couple of these and it seems they could be hard-coded to MIT

@watson watson force-pushed the graphql-codegen branch 3 times, most recently from 67e03d2 to 5a21b98 Compare March 27, 2020 10:43
@watson watson force-pushed the graphql-codegen branch 2 times, most recently from bd3520a to 033333e Compare April 27, 2020 12:41
@watson
Copy link
Contributor Author

watson commented Apr 27, 2020

Due to apollo-link being upgraded to v1.2.14 in this PR, we now get a new TypeScript error (from scripts/type_check.js) inside of said module:

ERROR x-pack/test failed
      node_modules/apollo-link/lib/types.d.ts:5:18 - error TS2430: Interface 'ExecutionResult<TData>' incorrectly extends interface 'ExecutionResult<ExecutionResultDataDefault>'.
        Types of property 'data' are incompatible.
          Type 'TData | null | undefined' is not assignable to type 'ExecutionResultDataDefault | undefined'.
            Type 'null' is not assignable to type 'ExecutionResultDataDefault | undefined'.

      5 export interface ExecutionResult<TData = {
                         ~~~~~~~

If I pin apollo-link to v1.2.13, the error goes away.

The change that causes this is this one: apollographql/apollo-link#1263

It appears that this is because the new module isn't compatible with GraphQL 13 which we are using. However, using Yarn resolutions to force apollo-link to be v1.2.13, isn't that maintainable, so I don't like it, but unless we can get apollo-link to release a new version that re-adds support for GraphQL 13, I don't see any other way short of upgrading our GraphQL version to v15.

Update: Found a way to get around this without using Yarn resolutions by modifying yarn.lock manually to ensure we used versions of apollo-link more in line with what's already in master

Fix two TypeScript errors that would occur when running `node
scripts/check_type`.

Both issues are related to the version of `apollo-link` being used.
@watson watson marked this pull request as ready for review April 28, 2020 15:20
@watson watson requested review from a team as code owners April 28, 2020 15:20
The problematic packages are no longer in the dependency tree. This
happended when I upgraded from v1.13.1 to 1.13.3.
@watson watson removed the request for review from peterschretlen April 28, 2020 16:03
@watson
Copy link
Contributor Author

watson commented Apr 28, 2020

@elasticmachine merge upstream

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Operations: LGTM

@watson
Copy link
Contributor Author

watson commented May 4, 2020

@elasticmachine merge upstream

@elasticmachine elasticmachine requested a review from a team as a code owner May 4, 2020 14:28
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

infra (logs/metrics) changes LGTM -> I don't think we use that script anymore, I'll ping the team about removing it if that's the case. Thanks!

@FrankHassanabad
Copy link
Contributor

When running from SIEM we got errors when I did:

cd ~/projects/kibana/x-pack/plugins/siem
node scripts/generate_types_from_graphql.js

And then when I ran a type check:

cd ~/projects/kibana
node scripts/type_check.js --project x-pack/tsconfig.json

And got a lot of errors (I won't post here unless you cannot reproduce)

Once those are fixed and any new different generated types are part of this PR, I will give it a test and if everything during runtime looks good I will give it a LGTM.

@watson watson added v7.9.0 and removed v7.8.0 labels May 28, 2020
@watson watson added v7.10.0 and removed v7.9.0 labels Aug 11, 2020
@watson
Copy link
Contributor Author

watson commented Sep 14, 2020

GraphQL is scheduled to be removed soon (tip from @XavierM), so I think I'll just close this PR, as the GraphQL code generator is being removed at the same time.

@watson watson closed this Sep 14, 2020
@watson watson deleted the graphql-codegen branch November 9, 2021 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants