-
Notifications
You must be signed in to change notification settings - Fork 1
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
[No-Jira] Upgrade Apollo Server 4 #1242
Conversation
Preview branch generated at https://upgrade-apollo-server-v4.d3dytjb8adxkk5.amplifyapp.com |
Bundle sizes [mpdx-react]Compared against 21a6276 No significant changes found |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for being willing to take this on! This needed to happen.
We should wait to push this live until the new year when everyone is back. I would hate for people to be called away from family to fix an error.
On a quick review, you should remove the old packages. They're also comments in the graphql-rest.page.ts
page that are related to these packages, which need to be removed.
apollo-server-micro
apollo-datasource-rest
You could use the depcheck
NPM package to find others that aren't used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good in my basic testing! In addition to removing the packages Daniel mentioned, you also need to run yarn
and commit the changes it makes.
You should also be able to remove |
@canac I ran |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking Great! I've reviewed some of the delete requests and they are working.
Thank you for removing the old dependencies and upgrading Apollo.
I would also wait to get Caleb's review, and then when you deploy it, ensure you are around for the following 4-6 hours to ensure nothing breaks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing the warnings anymore in the console. Great work with this!
3918291
to
2f08704
Compare
…om Apollo server 4
2f08704
to
3c94d91
Compare
This is good to go as far as I'm concerned! |
1a5dd7c
to
fa7974a
Compare
Description
Upgrade to Apollo Server 4
Checklist: