Enable support for React 18, bump Gatsby to v5 #3367
Conversation
… tables from "data view" page
|
Thanks for the pull request, @bradenmacdonald! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
openedx-webhooks
added
the
open-source-contribution
✅ Deploy Preview for paragon-openedx-v23 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
After reading through a ton of gatsbyjs/gatsby#36899 and trying a few things I found gatsbyjs/gatsby#36899 (comment) and managed to get the dev server running. bsmith@aximdev:~/code/paragon$ git diff
diff --git a/www/gatsby-config.mjs b/www/gatsby-config.mjs
index d0ab6c658e..aaf5daab39 100644
--- a/www/gatsby-config.mjs
+++ b/www/gatsby-config.mjs
@@ -130,4 +130,7 @@ export default {
// Match the location of the site on github pages if no path prefix is specified
pathPrefix: process.env.PATH_PREFIX || '',
plugins,
+ flags: {
+ DEV_SSR: true
+ },
};
Edit: I looked into this a bit more and found https://www.gatsbyjs.com/docs/reference/release-notes/v2.28/#feature-flags-in-gatsby-configjs. Setting the |
|
Re:
With this change - default: require.resolve(
- './src/templates/default-mdx-page-template.tsx',
- ),It seems the |
…rver Replaces/reverts "fix: limit parallelization of build to avoid OOM error during 'develop'"
✅ Deploy Preview for paragon-openedx-v22 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Yeah, me too. I tried a couple things based on the docs but didn't have much luck yet. There is some perhaps complication from the fact that Gatsby includes everything in Clearly, there is somewhere that we need to specify that |
|
So I managed to get the layout page to render, I had to comment some stuff in the |
|
@brian-smith-tcril Nice! I pulled in most of that change for now. It seems like pages like |
|
For some reason, either when I changed the target branch to |
Description
We are long overdue for supporting React 18. The challenge is that bumping to React 18 and testing with it on our docs site also requires bumping Gatsby from v4 to v5 and
gatsby-plugin-mdxfrom v3 to v5, which introduces a ton of breaking changes to how Gatsby and MDX work.These upgrades have been attempted before in #2774 and #2767 but so far nobody has been able to finish the work up. I'm thinking we need to bite the bullet sooner or later and get it sort out.
Deploy Preview
https://deploy-preview-3367--paragon-openedx-v23.netlify.app/
Status
Working:
npm run build)Not working:
userEventAPI, and then the tests will be passing again.)stylelintis not passing (tons of warnings)Manual testing
Run the gatsby site locally with the command
cd wwwandrm -rf .cache && NODE_OPTIONS=--max-old-space-size=16384 npm run startIssues and solutions
npm run start. I thought I had solved this withGATSBY_CPU_COUNT=4(f67d1f7) but it no longer seems to be helping (even if lowered down to 1). I think the memory usage depends on how much has been cached, so clearing the cache always requires a lot more memory. For now the only workaround is to run it withNODE_OPTIONS=--max-old-space-size=16384 npm run startbut allocating 16GB each time is not feasible.npm run build-docsseems to be working fine, and the "deploy preview" build of this PR works fine too.DEV_SSR/FAST_DEVwill avoid the issue.sassused via transitive dependencies. Will this version bump cause issues for Paragon consumers (if we change the sass code to the new version)? e.g. see 958f617sassmeans instead of e.g.map-get(...)we are encouraged to use@use "sass:map";andmap.get(...)- done via 958f617 but this may need to be reverted, see question above.importetc. have been suppressed for now: 6946d67 we probably can't address this properly without a major bootstrap upgrade{and<characters must now be escaped in markdown - solved with 0f6004fchildMdxAPI is no longer available -gatsby-plugin-mdxno longer allows rendering multiple MDX documents in one page. I worked around this by addingreact-markdownand rendering the props descriptions as plain markdown (they don't use MDX anyways) - 48aa670liveautomatically, so I had to addrehype-mdx-code-propsin 11b7605<PropsTable>as an MDX component was tricky because it requires some context from the parent JSX component, which was being inlined as a local variable resulting in lint warnings: "Do not define components during render". It turns out that the props tables in question, on this Data Views page are already present in the usual place at the end of the document anyways. So I think it's more consistent to just remove support for this element from that page (remove the duplicate props tables) and avoid having to re-engineerPropsTableto better support context for this one very questionable use case. Done with c92a1a2