[MDX] Migration to gatsby-plugin-mdx v4 with fix for onCreatePage trigger issue #4585

randychilau · 2023-07-20T05:46:47Z

Description

This PR is a second try migrating to gatsby-plugin-mdx v4 (and MDXv2) which fixes an issue discovered in the first try (PR #4532).

The issue was that mdx pages were being handled differently in v4 when the env variable CI is set to true activating customized onCreatePage and envCreatePage functions. CI=true is the default case for all Github Actions.

As a result, mdx sourced pages which used the children feature of v4 were triggering the customized onCreatePage function which is designed to handle non-mdx sourced pages. Adding a conditional check for path ending in .html resolves the issue.

before (taken from the checks in PR 4532):

redirect loop example at /programs/gsoc in netlify site preview.

after (taken from the checks in this PR):

no redirect loop example at /programs/gsoc in netlify site preview.

Notes for Reviewers

Added section to CONTRIBUTING.md file with reminders when working on gatsby-node.js and page creation/url/redirect.

🌺 Thanks for everyone's patience and quick action to report issues, revert the PR, and discuss possible solutions.

Signed commits

Yes, I signed my commits.

…4532-mdx2-upgrade" This reverts commit 8362b4e, reversing changes made to 4c2e586. Signed-off-by: Randy Lau <[email protected]>

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-07-20T06:03:05Z

🚀 Preview for commit d8e08ae at: https://64b8cdb5900f064ab12c3be6--layer5.netlify.app

Nikhil-Ladha · 2023-07-20T06:15:12Z

Everything looks, actually good this time.
Godd catch!

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-07-21T18:07:09Z

🚀 Preview for commit b8ec8f5 at: https://64bac90b210ccf0508d788eb--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-07-21T19:00:30Z

🚀 Preview for commit cb31caa at: https://64bad58a9338e10375d87a77--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-07-21T21:19:40Z

🚀 Preview for commit e4c1b8f at: https://64baf5fbcf89bc03a5636f85--layer5.netlify.app

Nikhil-Ladha · 2023-07-22T12:49:03Z

Please do discuss the PR on the Monday's website call if possible for you (time wise), and request for people to test out the preview. From, my side things look good and we can give this a go.

Nikhil-Ladha · 2023-07-22T12:49:52Z

gatsby-config.js

@@ -72,7 +78,7 @@ module.exports = {
                overrides: {
                  // or disable plugins
                  inlineStyles: false,
-                  cleanupIDs: false,
+                  cleanupIds: false,


I am guessing this resolves the cleanupIds error that we are sseing during the build, right?

Thanks for reviewing. Yes, that is correct.

Possible reason why:

The original property value came from Gatsby's docs.

but then I believe the warning started appearing in build logs after this package upgrade:
Bump @svgr/webpack from 6.5.1 to 8.0.1 #4467

and then when looking at the SVGO github's plugin chart, the property value had different casing:

After correcting the config settings with the SVGO casing the warning did not appear anymore.

Cheers!

@randychilau @Nikhil-Ladha, we're still seeing these messages while building. cleanupIds is enabled by default as @randychilau pointed out. Do we know why it's necessary to disable this minification? At first blush, minification is ideal. Was having this enabled, corrupting some images?

We should find that this mis-capitalization is once again corrected and that the warning messages disappear - 329de49#diff-b5e305780d9d473da97c61beab8bc36e5e8871b360942e4686c9b20d8c5d4cfaR75

Hi @leecalcote,

regarding the question:

"Do we know why it's necessary to disable this minification? At first blush, minification is ideal. Was having this enabled, corrupting some images?"

The cleanupIds option is disabled to prevent conflicts of multiple SVGs on the same page:

Note: SVGO does not produce unique IDs when using the cleanupIDs option; if you’re using SVGs that rely on IDs (e.g. to target template elements with use) and include multiple SVGs on the same page you may wish to disable the cleanupIDs option to prevent conflicts. (src)

If I recall correctly, the Layer5 site required cleanupIds to be false so that pages with multiple SVGs could have a unique ID for each SVG that could be targeted for transitioning between light and dark mode.

Cheers,
Randy

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-07-22T19:45:51Z

🚀 Preview for commit c594161 at: https://64bc318dc252e0225e126f4e--layer5.netlify.app

randychilau · 2023-07-24T19:20:11Z

Hi @Nikhil-Ladha,

We discussed this PR on today's Website's call, and asked the community to test the preview and I believe someone also built the branch without issues.

@leecalcote mentioned that this merge might be an issue for contributors who have experienced memory issues during the image-processing phase. Something I forgot to mention was that the memory issues were occurring during the Build HTML Renderering stage, and there were generally no issues during the image-processing stage. However every contributor's setup is different, and since I have not had issues with image-processing in the past, my build data points may be irrelevant.

Lee also mentioned potentially migrating to Gatsby 5 (which requires bump React 18 and includes a new webpack version) in this PR to see if the cold build time will come down.

So as of now I see a few options:

merge this PR
- accept the longer cold build time for now
- listen for any memory build issues from community
- create separate PR for migration to Gatsby 5
add to this PR a Gatsby 5 migration
- test to see build time reduces in Gatsby 5
- re-evaluate whether or not to merge this PR
hold this PR and create separate PR for migration to Gatsby 5 on the current site
- after PR for Gatsby 5 migration is tested and merged, revisit this PR for testing

My initial thought is if the choice is between option 2 or 3, then 3 would be the way to go since a Gatsby 5 migration can be done independent of MDX related upgrades and has its own benefits for the site.

Happy to hear thoughts on the above and how to proceed.

Cheers,
Randy

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-05T06:23:45Z

🚀 Preview for commit 7eddcd5 at: https://64f6c9073e38135fb2118291--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-05T18:37:52Z

🚀 Preview for commit 1e10f7a at: https://64f774de721d0f1471d942f0--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-05T19:00:08Z

🚀 Preview for commit d4861ea at: https://64f77a7a28f3e016e67a88f9--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-05T21:04:01Z

🚀 Preview for commit 3db1070 at: https://64f79759bfd6992f58637054--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-05T21:46:03Z

🚀 Preview for commit d64cd34 at: https://64f7a158767e580074fd3467--layer5.netlify.app

randychilau · 2023-09-05T23:22:02Z

Hi All,

Reviewed the latest updates/changes for compatibility with MDX v2, and added them to this PR.

This PR is waiting on the review and merging of PR #4646 (Gatsby v5). Afterwards, will merge those changes into this PR with the hope that migrating to Gatsby v5 (and new webpack) will address cold build times and memory requirements with MDX v2.

Thanks,
Randy

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-11T18:42:22Z

🚀 Preview for commit 6245012 at: https://64ff5ed6ecfe833e585869b2--layer5.netlify.app

Signed-off-by: Randy Lau <[email protected]>

l5io · 2023-09-20T19:13:18Z

🚀 Preview for commit 228a0e4 at: https://650b43fdbb800d32b676c3a6--layer5.netlify.app

stale · 2024-03-17T14:25:47Z

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.

vishalvivekm · 2024-07-29T13:03:22Z

@Aviral0702

randychilau added 3 commits July 19, 2023 21:23

Revert "Merge pull request layer5io#4580 from thisiskaransgit/revert-…

dd98947

…4532-mdx2-upgrade" This reverts commit 8362b4e, reversing changes made to 4c2e586. Signed-off-by: Randy Lau <[email protected]>

fix: .html check for already updated urls

322606a

Signed-off-by: Randy Lau <[email protected]>

feat: remind on page creation/url/redirect changes

d8e08ae

Signed-off-by: Randy Lau <[email protected]>

randychilau mentioned this pull request Jul 20, 2023

[MDX] Migration to gatsby-plugin-mdx v4 (requires migration to MDX v2) #4532

Merged

1 task

randychilau added 2 commits July 20, 2023 10:57

Merge 'master' into mdx2-upgrade-v3

9ba13f8

Signed-off-by: Randy Lau <[email protected]>

Merge branch 'master' into mdx2-upgrade-v3

b8ec8f5

fix: resize/optimize images with tiny-img

cb31caa

Signed-off-by: Randy Lau <[email protected]>

fix: remove InlineQuote from shortcodes

e4c1b8f

Signed-off-by: Randy Lau <[email protected]>

Nikhil-Ladha reviewed Jul 22, 2023

View reviewed changes

randychilau added 2 commits July 22, 2023 11:14

Merge branch 'master' into mdx2-upgrade-v3

a6872c5

fix: remove hashtag char from url

d92ab45

Signed-off-by: Randy Lau <[email protected]>

test: member mdx pages to 150

7eddcd5

Signed-off-by: Randy Lau <[email protected]>

randychilau added 6 commits September 4, 2023 23:32

test: limit member mdx pages to first 165

eaf0b61

Signed-off-by: Randy Lau <[email protected]>

fix: revert changes to reduce file size

f8e75dd

Signed-off-by: Randy Lau <[email protected]>

fix: update member mdx query and page creation

57de9b8

Signed-off-by: Randy Lau <[email protected]>

Merge branch 'master' into mdx2-upgrade-v3

021c435

Signed-off-by: Randy Lau <[email protected]>

fix: correct frontmatter image link

7c89b22

Signed-off-by: Randy Lau <[email protected]>

test: add image to blog post folder

1e10f7a

Signed-off-by: Randy Lau <[email protected]>

randychilau added 2 commits September 5, 2023 11:42

fix: update to react style format

efc5a32

Signed-off-by: Randy Lau <[email protected]>

test: remove mdx page restriction

d4861ea

Signed-off-by: Randy Lau <[email protected]>

randychilau added 2 commits September 5, 2023 13:46

revert: re-add google tag scripts to pages

e3c7b1f

Signed-off-by: Randy Lau <[email protected]>

revert: re-add MDXComponents to ssr

3db1070

Signed-off-by: Randy Lau <[email protected]>

fix: update to only include .mdx files

d64cd34

Signed-off-by: Randy Lau <[email protected]>

Merge branch 'master' into mdx2-upgrade-v3

6245012

Signed-off-by: Randy Lau <[email protected]>

Merge branch 'master' into mdx2-upgrade-v3

228a0e4

Signed-off-by: Randy Lau <[email protected]>

stale bot added the issue/stale Issue has not had any activity for an extended period of time label Mar 17, 2024

Ashparshp added the issue/willfix label Jul 16, 2024

stale bot removed the issue/stale Issue has not had any activity for an extended period of time label Jul 16, 2024

leecalcote requested review from Muhammed-Moinuddin, sudhanshutech and vishalvivekm July 22, 2024 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MDX] Migration to gatsby-plugin-mdx v4 with fix for onCreatePage trigger issue #4585

[MDX] Migration to gatsby-plugin-mdx v4 with fix for onCreatePage trigger issue #4585

randychilau commented Jul 20, 2023 •

edited

Loading

l5io commented Jul 20, 2023

Nikhil-Ladha commented Jul 20, 2023 •

edited

Loading

l5io commented Jul 21, 2023

l5io commented Jul 21, 2023

l5io commented Jul 21, 2023

Nikhil-Ladha commented Jul 22, 2023

Nikhil-Ladha Jul 22, 2023

randychilau Jul 22, 2023

leecalcote Aug 11, 2023

leecalcote Aug 11, 2023

randychilau Aug 25, 2023

l5io commented Jul 22, 2023

randychilau commented Jul 24, 2023 •

edited

Loading

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

randychilau commented Sep 5, 2023

l5io commented Sep 11, 2023

l5io commented Sep 20, 2023

stale bot commented Mar 17, 2024

vishalvivekm commented Jul 29, 2024

[MDX] Migration to gatsby-plugin-mdx v4 with fix for onCreatePage trigger issue #4585

Are you sure you want to change the base?

[MDX] Migration to gatsby-plugin-mdx v4 with fix for onCreatePage trigger issue #4585

Conversation

randychilau commented Jul 20, 2023 • edited Loading

l5io commented Jul 20, 2023

Nikhil-Ladha commented Jul 20, 2023 • edited Loading

l5io commented Jul 21, 2023

l5io commented Jul 21, 2023

l5io commented Jul 21, 2023

Nikhil-Ladha commented Jul 22, 2023

Nikhil-Ladha Jul 22, 2023

Choose a reason for hiding this comment

randychilau Jul 22, 2023

Choose a reason for hiding this comment

leecalcote Aug 11, 2023

Choose a reason for hiding this comment

leecalcote Aug 11, 2023

Choose a reason for hiding this comment

randychilau Aug 25, 2023

Choose a reason for hiding this comment

l5io commented Jul 22, 2023

randychilau commented Jul 24, 2023 • edited Loading

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

l5io commented Sep 5, 2023

randychilau commented Sep 5, 2023

l5io commented Sep 11, 2023

l5io commented Sep 20, 2023

stale bot commented Mar 17, 2024

vishalvivekm commented Jul 29, 2024

randychilau commented Jul 20, 2023 •

edited

Loading

Nikhil-Ladha commented Jul 20, 2023 •

edited

Loading

randychilau commented Jul 24, 2023 •

edited

Loading