Skip to content

Fix: support FormData object on fetch body#3417

Merged
bholmesdev merged 5 commits intomainfrom
fix/support-formdata-object
May 20, 2022
Merged

Fix: support FormData object on fetch body#3417
bholmesdev merged 5 commits intomainfrom
fix/support-formdata-object

Conversation

@bholmesdev
Copy link
Copy Markdown
Contributor

Changes

  • Resolves 🐛 BUG: FormData object causes fetch error #2741
  • Move node-fetch to external dependency for @astrojs/webapi bundle. This was causing strange issues with body parsing that I still haven't figured out. Still, since @astrojs/telemetry already pulls in node-fetch as a standard dependency, this shouldn't be a problem!
  • moving away from using node-fetch/src/index.js exposed type issues. A few pieces to resolve:
    • fetch no longer accepts URL as a resource type. Funny enough, you can still pass a URL for node-fetch to run toString() implicitly. Still, you get red squigglies in VS Code using URL today, so removing this type option is for consistency if anything.
    • FetchInit now uses node-fetch's body type in its type signature

Why not switch to undici like all the cool kids?

@FredKSchott (a cool kid) suggested using undici since it mirrors Node's incoming fetch standard. This seemed possible at first... until I discovered their node versioning requirements. In short: they don't expose fetch or Request for node versions below 16.5. Also poked the source code to confirm. This would mean dropping Node 14.X support, which feels a bit too far as we approach 1.0. Keep it in mind for the future though!

Testing

N/A

Docs

N/A

Copy link
Copy Markdown
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

lgtm

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 20, 2022

🦋 Changeset detected

Latest commit: 3174bde

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@astrojs/webapi Minor
astro Patch
@astrojs/netlify Patch
@astrojs/node Patch
@astrojs/vercel Patch
astro-scripts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Copy Markdown
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

LGTM! I would change the changeset to minor for webapi though.

Comment thread .changeset/serious-vans-smell.md Outdated
@bholmesdev bholmesdev merged commit 4de53ec into main May 20, 2022
@bholmesdev bholmesdev deleted the fix/support-formdata-object branch May 20, 2022 21:26
@github-actions github-actions Bot mentioned this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 BUG: FormData object causes fetch error

3 participants