-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Gatsby] Migration to v5 #4646
[Gatsby] Migration to v5 #4646
Conversation
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
🚀 Preview for commit 3bb83a7 at: https://64c305f1234a480d8a3d2506--layer5.netlify.app |
LGTM , |
Hi @Ghat0tkach. Thanks for taking the time to check the PR preview for issues, it is very much appreciated. I have recently discovered some issues with this PR that will require some additional updates before it will be ready for merging. Thanks again! Cheers, |
Signed-off-by: Randy Lau <[email protected]>
🚀 Preview for commit dd155a7 at: https://64ff5c0861818b42499fdbbd--layer5.netlify.app |
Signed-off-by: Randy Lau <[email protected]>
Signed-off-by: Randy Lau <[email protected]>
🚀 Preview for commit dc7875f at: https://650b22944c2d9a1ac3944654--layer5.netlify.app |
@randychilau's efforts here are commendable. Just fantastic. @Udit-takkar do you mind terribly to give this a final once-over? |
@Mohith234, noting the "Notes for Reviewers" section, are you able to checkout this branch build and run w/o issue? |
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.
@Mohith234, noting the "Notes for Reviewers" section, are you able to checkout this branch build and run w/o issue?
Yes, I was able to run w/o any issues, but just got some logs saying ERROR. Attaching a screenshot for reference below.
P.S. The site was running successfully
Hi @Mohith234, Thanks for taking the time to help on this PR and posting the screenshot. The error message is related to outdated MDX packages. This issue is addressed in a separate PR #4585 (which is awaiting Gatsby v5 as discussed here). Cheers, |
@VamshiReddy02, do you mind testing this PR as well? |
Signed-off-by: Lee Calcote <[email protected]>
🚀 Preview for commit ef1e69a at: https://650e33da42629256d961bc1f--layer5.netlify.app |
Thanks much for this, @VamshiReddy02! 👏 |
@randychilau As mentioned here - Notes for Reviewers |
Hi @Chadha93, Thanks for your message. Yes, if this PR is merged, I will then update PR #4585 (which includes upgrades to MDX v2 and MDX related packages, as well as changes for MDX v2 compatibility) and have it available for review the same day (hopefully). Since merging this PR #4646 does require users to temporarily add the "--legacy-peer-deps" flag to install the package changes until PR #4585 merges, I am not sure what are the best practices to minimize confusion/disruption. Should a couple of days/weekend be scheduled to try merging both PRs, should there be any messaging to the community? and what impact will this have on current PRs? I defer to you, Lee, and the maintainers on what is best. Cheers, |
Hi @randychilau, Thanks for this descriptive response. This really helps. Answering your question - Although, both of the PRs should be merged at the same time when ready, to avoid conflicts for contributors to install the package changes, if merging this PR, is strictly required before updating the other PR, then lets merge this and until then update our contributing.md file with the instruction to add "--legacy-peer-deps" flag. In both of the cases, we are good. Cheers, |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Currently I am facing the following issues after pulling the latest changes from the master.
|
Description
This PR migrates the site from Gatsby v4 to v5 which requires React v18.
Update 09/20/2023: The latest package.json changes from the master branch can be seen here.
Additional Plugins Added/Updated:
Plugins Removed (either not used as detected by depCheck or removed for redundancy with React 18 functionality):
gatsby-plugin-loadable-components-ssrNotes for Reviewers
It is temporarily required to install with
npm install --legacy-peer-deps
due to incompatibility with mdxv1 (a separate PR to be merged if/when this PR is merged).A Github Page hosted preview is availalble here.
For plugins removed, please notify me if there is missing functionality due to removal.
Signed commits