Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Updated dependencies and added Renovate bot to auto update #103

Merged
merged 46 commits into from
Aug 5, 2020
Merged

Updated dependencies and added Renovate bot to auto update #103

merged 46 commits into from
Aug 5, 2020

Conversation

Tielem
Copy link
Contributor

@Tielem Tielem commented Aug 3, 2020

As discussed in #100, I took a stab at making some version changes to the repository

Concretely:

  • Set up Renovatebot to (1) auto update through PR's, (2) auto merge its PR's for anything but majors when all CI actions succeed, (3) keep graphql on version <=14 (so not 15 due to Gatsby incompatibility)
  • I changed one test to make it platform independent (takes in options and returns a function that runs codegen for the schema)
  • I replaced @graphql-tookit by @graphql-tools because it was deprecated
  • Updating eslint and prettier introduced a bunch of linting errors which were resolved
  • Updating ts-checker-webpack-plugin required a configuration signature to be changed
  • I got rid of the gatsby/graphql dependency. Mainly because you also had a regular graphql dependency in the project where you used it. I also don't think it works as expected/desired. When the gatsby/graphql dependency is set to v15, gatsby projects which use the plugin don't build correctly
  • I set fileName as an optional configuration parameter of gatsby-plugin-graphql-codegen. It isn't always specified in the examples and typescript complained about the combination of default config and given configuration, since there is a value in the default configuration for fileName
  • Deleted some old/non used lockfiles

I also made some changes to the Github Actions:

  • I merged the build, lint, test in one action. In hindsight, this wasn't really needed. I am used to this workflow from previous projects, but I can revert this if desired
  • I set the tests to run on node 14
  • Updated cache plugin to v2

Some changes which may be desired, but I didn't make:

  • The code relies on the generated graphql types. However, these types have linting errors. If you run yarn build, then yarn lint you will see linting errors. Maybe the generated types should get excluded from eslint? Or, if possible, when eslint is installed it should --fix the types on generation?
  • The current build doesn't build the gatsby-starter-blog-ts. If this is desired, its script should be renamed from build-site to build

After merging this PR, you will still have to add the repository to Renovate bot and disable dependabot. As said, it will PR and merge most dependencies automatically unless (1) it is a major update or (2) the build fails during update. If Gatsby get their stuff together, the <= 14 restriction for graphql can be removed

Let me know what you think!

Tielem and others added 30 commits August 1, 2020 13:40
…ugh PR's, (2) update both dependencies and devDependencies, and (3) limits graphql to major 14
@d4rekanguok
Copy link
Owner

@Tielem, this is awsome, thank you so much for putting in time bring this repo up-to-date ❤️. This is a lot of work!!

All looks good to me, though I'm worry about not using gatsby/graphql.

We started out using just graphql, but quickly run into conflicting errors. Gatsby injects additional stuff into its graphql instance), Gatsby codebase with graphql@15 wil not work, even without this plugin installed (#82, #101).

Because of that, I think switching away from gatsby/graphql needs more testing & consideration, but it's beside the scope of this PR — would you mind reverting back to only using gatsby/graphql? I'll merge this in after that & we can continue the discussion on graphql version in another issue.

Let me know what you think — and again thank you for the work you've done here!

@Tielem
Copy link
Contributor Author

Tielem commented Aug 4, 2020

I reverted the commit where I removed gatsby/graphql
Keep in mind, it is only being used in graphql-codegen.config.ts, not in gatsby-node.ts of the plugin (where the normal, local graphql dependency already is used)

What do you want me to do with the Github Actions files? Should I revert those too, with some changes, so you have the build, lint, test separately?

@d4rekanguok
Copy link
Owner

Keep in mind, it is only being used in graphql-codegen.config.ts, not in gatsby-node.ts of the plugin (where the normal, local graphql dependency already is used)

If I'm not mistaken graphql is only used in gatsby-node.ts for typing purpose, which will be stripped off anyway.

What do you want me to do with the Github Actions files? Should I revert those too, with some changes, so you have the build, lint, test separately?

I like that we're running all test in one action, that's really neat — thank you!

@Tielem
Copy link
Contributor Author

Tielem commented Aug 4, 2020

If I'm not mistaken graphql is only used in gatsby-node.ts for typing purpose, which will be stripped off anyway.

This is correct, it's only used to type some arguments.

@d4rekanguok d4rekanguok merged commit fdd5dc0 into d4rekanguok:master Aug 5, 2020
@d4rekanguok
Copy link
Owner

Thank you again @Tielem!

@zsherman
Copy link

zsherman commented Aug 26, 2020

Is there a plan to release these changes now that they're merged? Depending on old versions of graphql and graphql-toolkit is causing errors for us on newer installations. Thanks!

@d4rekanguok
Copy link
Owner

@zsherman 🤦‍♀️ I forgot to publish a new version, my bad

@d4rekanguok
Copy link
Owner

Hhhhm I run into odd local errors when using the plugins locally (even though CI tests passed). Will need to look more

 ERROR #11321  PLUGIN

"gatsby-plugin-ts" threw an error while running the onPostBootstrap lifecycle:

Could not obtain introspection result, received: {"error":{"code":"503","message":"This deployment is no longer available."}}



  Error: Could not obtain introspection result, received: {"error":{"code":"503","message":"This deployment is no longer available."}}
  
  - introspect.js:17 getSchemaFromIntrospection
    /Users/[]/gatsby-plugin-ts/dist/wrap/src/introspect.js:17:15
  
  - introspect.js:26 Object.introspectSchema
    /Users/[]/gatsby-plugin-ts/dist/wrap/src/introspect.js:26:12
  
  - task_queues.js:97 processTicksAndRejections
    internal/process/task_queues.js:97:5

@zsherman
Copy link

Ok let me know if I can help!

@d4rekanguok
Copy link
Owner

@zsherman I would appreciate any help, please let me know if you'd like to take a stab! 🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants