-
Notifications
You must be signed in to change notification settings - Fork 223
Use native FormData for bulk mutation variable file uploads
#6756
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
Conversation
The form-data package triggers a deprecation warning in node-fetch 3.x when passed as a request body. Switch to native FormData + Blob for the staged upload in bulk operations to avoid the warning. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
FormData for bulk operation uploads
FormData for bulk operation uploadsFormData for bulk operation staged file uploads
FormData for bulk operation staged file uploadsFormData for bulk mutation variable file uploads
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3635 tests passing in 1426 suites. Report generated by 🧪jest coverage report action from f08eeb4 |
| } from '../../api/graphql/bulk-operations/generated/staged-uploads-create.js' | ||
| import {adminRequestDoc} from '@shopify/cli-kit/node/api/admin' | ||
| import {AdminSession} from '@shopify/cli-kit/node/session' | ||
| import {formData, fetch} from '@shopify/cli-kit/node/http' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this package from cli-kit now? I'd love to see a follow up PR if that is the case, reducing the bundle! Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yeah :) Claude did that at first, but I asked him to minimize the diff and make this PR smaller, because I wasn't sure if removing it entirely was productive. I can absolutely do that in a follow-up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aw, looks like we can't actually remove form-data from cli-kit yet.
I started sketching out the PR to fully remove it (link), but I discovered @shopify/oxygen-cli still imports formData from @shopify/cli-kit/node/http. (See here.)
We'd need to update oxygen-cli to use native FormData first, then revisit this. 😭
…loads Use native `FormData` for bulk mutation variable file uploads
Small improvement related to https://github.com/shop/issues-api-foundations/issues/1047. Found it while testing the command looking for bugs (https://github.com/shop/issues-api-foundations/issues/1081).
Summary
The
form-datanpm package triggers a deprecation warning when passed as a request body to node-fetch 3.x. This PR fixes it by switching bulk operation file uploads to use nativeFormData+Blobinstead of theform-datapackage.Tophatting
On
main, running a bulk mutation:On this branch: