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

fix: don't fail entire build when gatsby-config fails to be imported #562

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Feb 10, 2023

Summary

Build plugin tries to load gatsby-config.js to inspect list of gatsby plugins, ensure that gatsby-plugin-netlify is used and that version is compatible etc. It's currently only meant (?) to generate warning logs when it finds incompatibilities, but incompatibilities themselves are not causing builds to fail.

Current config loading logic does fail entire build if it fails to require gatsby-config.js and this might happen if user is authoring it in ESM, still uses .js extension (and no type: "module" in package json) and uses tools like esm, @babel/register etc to support that syntax in the build command.

The config checks in this plugin currently also are not handling some other gatsby-config variants that Gatsby itself support now:

With above in mind (plugins validation is already not happening in some cases) + @netlify/plugin-gatsby being installed automatically it might make sense to make failures to load config not fatal (at least for now) (?)

Test plan

  1. Added e2e test case with sample site using gatsby@3, with gatsby-config.js using ESM and using following build command NODE_OPTIONS="-r esm" gatsby build.
  2. Confirm test case fails with current version of build plugin ( tests fail on first commit in this PR - https://github.com/netlify/netlify-plugin-gatsby/actions/runs/4145358783/jobs/7169610777 )
  3. Confirm test case passes with code changes in this PR ( tests pass on second commit in this PR - https://github.com/netlify/netlify-plugin-gatsby/actions/runs/4145916392/jobs/7170917995 )

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Fixes #559

Standard checks:

  • Check the Deploy Preview's Demo site for your PR's functionality
  • Add docs when necessary

🧪 Once merged, make sure to update the version if needed and that it was
published correctly.

@netlify
Copy link

netlify bot commented Feb 10, 2023

Deploy Preview for netlify-plugin-gatsby-demo ready!

Name Link
🔨 Latest commit 6b5a79e
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-gatsby-demo/deploys/640ef6c4b10eb80008017f3c
😎 Deploy Preview https://deploy-preview-562--netlify-plugin-gatsby-demo.netlify.app/blog
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pieh pieh force-pushed the fix/dont-fail-build-if-config-fails-to-import branch from d5ad5b2 to 94f3549 Compare February 10, 2023 16:48
@pieh pieh marked this pull request as ready for review February 10, 2023 17:38
@pieh pieh requested a review from a team February 10, 2023 17:38
@pieh pieh changed the title [WIP] fix: don't fail entire build when gatsby-config fails to be imported fix: don't fail entire build when gatsby-config fails to be imported Feb 10, 2023
@pieh pieh added the automerge label Mar 13, 2023
@netlify
Copy link

netlify bot commented Mar 13, 2023

Deploy Preview for netlify-plugin-gatsby-demo-v5 ready!

Name Link
🔨 Latest commit 6b5a79e
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-gatsby-demo-v5/deploys/640ef6c44c21df00080b03a5
😎 Deploy Preview https://deploy-preview-562--netlify-plugin-gatsby-demo-v5.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kodiakhq kodiakhq bot merged commit 007c823 into main Mar 13, 2023
@kodiakhq kodiakhq bot deleted the fix/dont-fail-build-if-config-fails-to-import branch March 13, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Netlify Gatsby plugin causes build failures on Gatsby 3.4 site
2 participants