Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/remove-form-data.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/app': patch
---

Remove form-data dependency and use native Node FormData instead
16 changes: 12 additions & 4 deletions packages/app/src/cli/services/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {AssetUrlSchema, DeveloperPlatformClient} from '../utilities/developer-pl
import {MinimalAppIdentifiers} from '../models/organization.js'
import {joinPath} from '@shopify/cli-kit/node/path'
import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver'
import {formData, fetch} from '@shopify/cli-kit/node/http'
import {fetch} from '@shopify/cli-kit/node/http'
import {fileSize, readFileSync} from '@shopify/cli-kit/node/fs'
import {AbortError} from '@shopify/cli-kit/node/error'
import {writeFile} from 'fs/promises'
Expand Down Expand Up @@ -44,10 +44,18 @@ export async function uploadToGCS(signedURL: string, filePath: string) {
`Check the asset paths in your extension configuration — a misconfigured source can pull in much more than intended. Exclude large files or directories from your bundle, then try again.`,
)
}
const form = formData()
const buffer = readFileSync(filePath)
form.append('my_upload', buffer)
await fetch(signedURL, {method: 'put', body: buffer, headers: form.getHeaders()}, 'slow-request')
await fetch(
signedURL,
{
method: 'put',
body: buffer,
headers: {
'content-type': 'multipart/form-data; boundary=---shopify-cli-upload-boundary---',
},
},
'slow-request',
)
}

/**
Expand Down
15 changes: 4 additions & 11 deletions packages/app/src/cli/services/deploy/upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {testApp, testDeveloperPlatformClient} from '../../models/app/app.test-da
import {AppDeploySchema, AppDeployVariables} from '../../api/graphql/app_deploy.js'
import {describe, expect, test, vi} from 'vitest'
import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'
import {formData} from '@shopify/cli-kit/node/http'
import {joinPath} from '@shopify/cli-kit/node/path'

vi.mock('@shopify/cli-kit/node/http')
Expand All @@ -16,8 +15,7 @@ describe('uploadExtensionsBundle', () => {
test('calls a mutation on partners', async () => {
await inTemporaryDirectory(async (tmpDir) => {
// Given
const mockedFormData = {append: vi.fn(), getHeaders: vi.fn()}
vi.mocked<any>(formData).mockReturnValue(mockedFormData)

const developerPlatformClient = testDeveloperPlatformClient()

// When
Expand Down Expand Up @@ -62,8 +60,7 @@ describe('uploadExtensionsBundle', () => {
test('calls a mutation on partners with a message and a version', async () => {
await inTemporaryDirectory(async (tmpDir) => {
// Given
const mockedFormData = {append: vi.fn(), getHeaders: vi.fn()}
vi.mocked<any>(formData).mockReturnValue(mockedFormData)

const developerPlatformClient = testDeveloperPlatformClient()

// When
Expand Down Expand Up @@ -111,8 +108,7 @@ describe('uploadExtensionsBundle', () => {

test('calls a mutation on partners when there are no extensions', async () => {
const developerPlatformClient = testDeveloperPlatformClient()
const mockedFormData = {append: vi.fn(), getHeaders: vi.fn()}
vi.mocked<any>(formData).mockReturnValue(mockedFormData)

// When
await uploadExtensionsBundle({
appManifest,
Expand Down Expand Up @@ -222,8 +218,6 @@ describe('uploadExtensionsBundle', () => {
deploy: (_input: AppDeployVariables) => Promise.resolve(errorResponse),
})

const mockedFormData = {append: vi.fn(), getHeaders: vi.fn()}
vi.mocked<any>(formData).mockReturnValue(mockedFormData)
// When
await writeFile(joinPath(tmpDir, 'test.zip'), '')

Expand Down Expand Up @@ -327,8 +321,7 @@ describe('uploadExtensionsBundle', () => {
const developerPlatformClient = testDeveloperPlatformClient({
deploy: (_input: AppDeployVariables) => Promise.resolve(errorResponse),
})
const mockedFormData = {append: vi.fn(), getHeaders: vi.fn()}
vi.mocked<any>(formData).mockReturnValue(mockedFormData)

await writeFile(joinPath(tmpDir, 'test.zip'), '')

// When
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
testWebhookExtensions,
} from '../../../../models/app/app.test-data.js'
import {getUploadURL, writeManifestToBundle} from '../../../bundle.js'
import {formData} from '@shopify/cli-kit/node/http'
import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest'
import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort'
import {flushPromises} from '@shopify/cli-kit/node/promises'
Expand Down Expand Up @@ -90,7 +89,6 @@ describe('pushUpdatesForDevSession', () => {
let devSessionStatusManager: DevSessionStatusManager

beforeEach(() => {
vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any)
stdout = {write: vi.fn()}
stderr = {write: vi.fn()}
developerPlatformClient = testDeveloperPlatformClient()
Expand Down Expand Up @@ -537,7 +535,7 @@ describe('pushUpdatesForDevSession', () => {

test('assetsURL is only generated if affected extensions have assets', async () => {
// Given
vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any)

// Mock readdir to return that a folder for the extension assets exists
vi.mocked(readdir).mockResolvedValue(['other-folders', 'ui-extension-uid'])
vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url')
Expand Down Expand Up @@ -568,7 +566,7 @@ describe('pushUpdatesForDevSession', () => {

test('assetsURL is always generated for create, even if there are no assets', async () => {
// Given
vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any)

vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url')

// When
Expand Down
3 changes: 1 addition & 2 deletions packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@
"@graphql-typed-document-node/core": "3.2.0",
"@iarna/toml": "2.2.5",
"@oclif/core": "4.5.3",
"@shopify/toml-patch": "0.3.0",
"@opentelemetry/api": "1.9.0",
"@opentelemetry/core": "1.30.0",
"@opentelemetry/exporter-metrics-otlp-http": "0.57.0",
"@opentelemetry/resources": "1.30.0",
"@opentelemetry/sdk-metrics": "1.30.0",
"@shopify/toml-patch": "0.3.0",
"@types/archiver": "5.3.2",
"ajv": "8.18.0",
"ansi-escapes": "6.2.1",
Expand All @@ -129,7 +129,6 @@
"fast-glob": "3.3.3",
"figures": "5.0.0",
"find-up": "6.3.0",
"form-data": "4.0.4",
"fs-extra": "11.1.0",
"gradient-string": "2.0.2",
"graphql": "16.10.0",
Expand Down
16 changes: 10 additions & 6 deletions packages/cli-kit/src/private/node/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ import {sleepWithBackoffUntil} from './sleep-with-backoff.js'
import {outputDebug} from '../../public/node/output.js'
import {recordRetry} from '../../public/node/analytics.js'

import {Headers} from 'form-data'
import {ClientError} from 'graphql-request'

import {performance} from 'perf_hooks'

export interface AnyHeaders {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
forEach: (callback: (value: any, key: any) => void) => void
get?: (name: string) => string | null
}

export type API = 'admin' | 'storefront-renderer' | 'partners' | 'business-platform' | 'app-management'

export const allAPIs: API[] = ['admin', 'storefront-renderer', 'partners', 'business-platform', 'app-management']
Expand Down Expand Up @@ -137,7 +141,7 @@ export function isNetworkError(error: unknown): boolean {
return false
}

async function runRequestWithNetworkLevelRetry<T extends {headers: Headers; status: number}>(
async function runRequestWithNetworkLevelRetry<T extends {headers: AnyHeaders; status: number}>(
requestOptions: RequestOptions<T>,
): Promise<T> {
if (!requestOptions.useNetworkLevelRetry) {
Expand Down Expand Up @@ -166,7 +170,7 @@ async function runRequestWithNetworkLevelRetry<T extends {headers: Headers; stat
throw lastSeenError
}

async function makeVerboseRequest<T extends {headers: Headers; status: number}>(
async function makeVerboseRequest<T extends {headers: AnyHeaders; status: number}>(
requestOptions: RequestOptions<T>,
): Promise<VerboseResponse<T>> {
const t0 = performance.now()
Expand Down Expand Up @@ -266,7 +270,7 @@ function errorsIncludeStatus429(error: ClientError): boolean {
return error.response.errors?.some((error) => error.extensions?.code === '429') ?? false
}

export async function simpleRequestWithDebugLog<T extends {headers: Headers; status: number}>(
export async function simpleRequestWithDebugLog<T extends {headers: AnyHeaders; status: number}>(
requestOptions: RequestOptions<T>,
errorHandler?: (error: unknown, requestId: string | undefined) => unknown,
): Promise<T> {
Expand Down Expand Up @@ -329,7 +333,7 @@ ${result.sanitizedHeaders}
* @param retryOptions - Options for the retry
* @returns The response from the request
*/
export async function retryAwareRequest<T extends {headers: Headers; status: number}>(
export async function retryAwareRequest<T extends {headers: AnyHeaders; status: number}>(
requestOptions: RequestOptions<T>,
errorHandler?: (error: unknown, requestId: string | undefined) => unknown,
retryOptions: {
Expand Down
2 changes: 0 additions & 2 deletions packages/cli-kit/src/public/node/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {platformAndArch} from './os.js'
import {afterAll, afterEach, beforeAll, beforeEach, describe, expect, test, vi} from 'vitest'
import {setupServer} from 'msw/node'
import {delay, http, HttpResponse} from 'msw'
import FormData from 'form-data'

const DURATION_UNTIL_ABORT_IS_SEEN = 100

Expand Down Expand Up @@ -80,7 +79,6 @@ describe('formData', () => {
test('make an empty form data object', () => {
const res = formData()
expect(res).toBeInstanceOf(FormData)
expect(res.getLengthSync()).toBe(0)
})
})

Expand Down
1 change: 0 additions & 1 deletion packages/cli-kit/src/public/node/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {httpsAgent, sanitizedHeadersOutput} from '../../private/node/api/headers
import {NetworkRetryBehaviour, simpleRequestWithDebugLog} from '../../private/node/api.js'
import {DEFAULT_MAX_TIME_MS} from '../../private/node/sleep-with-backoff.js'

import FormData from 'form-data'
import nodeFetch, {RequestInfo, RequestInit, Response} from 'node-fetch'

export {FetchError, Request, Response} from 'node-fetch'
Expand Down
Loading
Loading