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

upgrade gatsby #8334

Merged
merged 3 commits into from
Aug 25, 2022
Merged

upgrade gatsby #8334

merged 3 commits into from
Aug 25, 2022

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Aug 22, 2022

boom 💥

  • conflicts resolved
  • no new security vulns
  • tests passing

I've labelled this blocker because I'd quite like to merge this before dependabot pops up on Friday and we get a bunch of merge conflicts on package-lock.json, although if I do need to do it again, I can.

@shields-ci
Copy link

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 9eac07f

@chris48s chris48s added dependencies Related to dependency updates blocker PRs and epics which block other work labels Aug 22, 2022
@chris48s chris48s changed the title WIP Upgrade gatsby upgrade gatsby Aug 22, 2022
@chris48s chris48s marked this pull request as ready for review August 22, 2022 18:10
Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

I think the dictionary needs an updated definition for "persistence" to include this 😆

@calebcartwright
Copy link
Member

Also, I think I get the gist of the skipLibCheck tsconfig option, but would you mind giving me a quick recap of the impact and implications for us here?

@chris48s
Copy link
Member Author

The issue we've got with gatsby/express-graphql/graphql is basically around type checking/incompatibility with tsc. Using skipLibCheck disables all type checking in .d.ts files. See https://bobbyhadz.com/blog/typescript-disable-type-checking-node-modules . There is a tradeoff here. We do lose some type safety not only within node_modules but also to a lesser extent within our own codebase: It does also disable https://github.com/badges/shields/blob/master/frontend/types/assets.d.ts and https://github.com/badges/shields/blob/master/frontend/types/mapbox__react-click-to-select/index.d.ts .

@calebcartwright
Copy link
Member

Thanks for that. Agreed the tradeoffs are worth it here, we've been in dependency upgrade hell for too long

@chris48s chris48s merged commit 08af9ea into badges:master Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker PRs and epics which block other work dependencies Related to dependency updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants