Skip to content

Conversation

@ericapisani
Copy link
Contributor

Summary

In the initial release of #376 , the minimum version of node was increased to v14.17.0 because that was when the AbortController that's used in the pre-warm requests to the lambdas in the onSuccess was first included within the Node global object.

However, AbortController is controlled by a feature flag in v14, leading to errors of it not existing when building sites with the plugin.

Changes

  • Add an AbortController polyfill to support Node v14.17+
  • Remove references to globalThis - this was incorrectly added in the initial implementation
  • Update imports from node:process to process. This was being raised as an error in Node v14

Test plan

Deploy previews aren't how one would reproduce the error. To reproduce (in local environment):

  • Install Node v14.17.0
  • Attempt to build the demo site with the GATSBY_EXCLUDE_DATASTORE_FROM_BUNDLE environment variable enabled

If one were to do this prior to these changes, you should see an error that looks like globalThis.AbortController is not a constructor.

After these changes, the site should correctly build and deploy

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

bird

@ericapisani ericapisani requested review from a team and ascorbic May 31, 2022 18:10
@ericapisani ericapisani self-assigned this May 31, 2022
@netlify
Copy link

netlify bot commented May 31, 2022

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

Name Link
🔨 Latest commit 6c63a31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-gatsby-demo/deploys/62965a1cf7817200097217a4
😎 Deploy Preview https://deploy-preview-391--netlify-plugin-gatsby-demo.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.

Copy link
Contributor

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this locally, but it looks good to me

Copy link
Contributor

@sarahetter sarahetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and it works as described!

@ascorbic
Copy link
Contributor

ascorbic commented Jun 1, 2022

:shipit:

@kodiakhq kodiakhq bot merged commit db412a2 into main Jun 1, 2022
@kodiakhq kodiakhq bot deleted the ep/polyfill-abortcontroller branch June 1, 2022 11:02
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.

4 participants