From 6cec77f10c50888c058cc6d7ec51618220f96d6f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 13 Dec 2023 16:19:47 +0100 Subject: [PATCH 01/14] wip --- node-src/lib/upload.ts | 6 ++++-- node-src/lib/utils.ts | 2 -- node-src/tasks/upload.test.ts | 8 ++++---- node-src/tasks/upload.ts | 5 +++-- node-src/types.ts | 1 + node-src/ui/messages/errors/fatalError.stories.ts | 3 ++- node-src/ui/messages/info/storybookPublished.stories.ts | 1 + 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index f5ad83571..e9d354c93 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -84,7 +84,8 @@ export async function uploadBuild( UploadBuildMutation, { buildId: ctx.announcedBuild.id, - files: files.map(({ contentLength, targetPath }) => ({ + files: files.map(({ contentHash, contentLength, targetPath }) => ({ + contentHash, contentLength, filePath: targetPath, })), @@ -178,7 +179,8 @@ export async function uploadMetadata(ctx: Context, files: FileDesc[]) { UploadMetadataMutation, { buildId: ctx.announcedBuild.id, - files: files.map(({ contentLength, targetPath }) => ({ + files: files.map(({ contentHash, contentLength, targetPath }) => ({ + contentHash, contentLength, filePath: targetPath, })), diff --git a/node-src/lib/utils.ts b/node-src/lib/utils.ts index 7b36c0037..dd783734f 100644 --- a/node-src/lib/utils.ts +++ b/node-src/lib/utils.ts @@ -34,8 +34,6 @@ export const activityBar = (n = 0, size = 20) => { return `[${track.join('')}]`; }; -export const baseStorybookUrl = (url: string) => url?.replace(/\/iframe\.html$/, ''); - export const rewriteErrorMessage = (err: Error, message: string) => { try { // DOMException doesn't allow setting the message, so this might fail diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index 95b34985a..4bf979810 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -306,8 +306,8 @@ describe('uploadStorybook', () => { expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { buildId: '1', files: [ - { contentLength: 42, filePath: 'iframe.html' }, - { contentLength: 42, filePath: 'index.html' }, + { contentHash: undefined, contentLength: 42, filePath: 'iframe.html' }, + { contentHash: undefined, contentLength: 42, filePath: 'index.html' }, ], }); expect(http.fetch).toHaveBeenCalledWith( @@ -468,8 +468,8 @@ describe('uploadStorybook', () => { expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { buildId: '1', files: [ - { contentLength: 42, filePath: 'iframe.html' }, - { contentLength: 42, filePath: 'index.html' }, + { contentHash: undefined, contentLength: 42, filePath: 'iframe.html' }, + { contentHash: undefined, contentLength: 42, filePath: 'index.html' }, ], zip: true, }); diff --git a/node-src/tasks/upload.ts b/node-src/tasks/upload.ts index 93903c3be..cacb17729 100644 --- a/node-src/tasks/upload.ts +++ b/node-src/tasks/upload.ts @@ -22,7 +22,7 @@ import { success, hashing, } from '../ui/tasks/upload'; -import { Context, Task } from '../types'; +import { Context, FileDesc, Task } from '../types'; import { readStatsFile } from './read-stats-file'; import bailFile from '../ui/messages/warnings/bailFile'; import { findChangedPackageFiles } from '../lib/findChangedPackageFiles'; @@ -205,7 +205,8 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { transitionTo(preparing)(ctx, task); const files = ctx.fileInfo.paths - .map((path) => ({ + .map((path) => ({ + ...(ctx.fileInfo.hashes && { contentHash: ctx.fileInfo.hashes[path] }), contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === path).contentLength, localPath: join(ctx.sourceDir, path), targetPath: path, diff --git a/node-src/types.ts b/node-src/types.ts index 5014f3205..4eee98335 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -354,6 +354,7 @@ export interface Stats { } export interface FileDesc { + contentHash?: string; contentLength: number; localPath: string; targetPath: string; diff --git a/node-src/ui/messages/errors/fatalError.stories.ts b/node-src/ui/messages/errors/fatalError.stories.ts index f77db2c3b..77a8a4f6a 100644 --- a/node-src/ui/messages/errors/fatalError.stories.ts +++ b/node-src/ui/messages/errors/fatalError.stories.ts @@ -47,9 +47,10 @@ const context = { build: { id: '5ec5069ae0d35e0022b6a9cc', number: 42, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', webUrl: 'https://www.chromatic.com/build?appId=5d67dc0374b2e300209c41e7&number=1400', }, - storybookUrl: 'https://pfkaemtlit.tunnel.chromaticqa.com/', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', }; const stack = `Error: Oh no! diff --git a/node-src/ui/messages/info/storybookPublished.stories.ts b/node-src/ui/messages/info/storybookPublished.stories.ts index 9dbee8d3a..72826e373 100644 --- a/node-src/ui/messages/info/storybookPublished.stories.ts +++ b/node-src/ui/messages/info/storybookPublished.stories.ts @@ -19,6 +19,7 @@ export const StorybookPrepared = () => errorCount: undefined, componentCount: 5, specCount: 8, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', }, storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', } as any); From 053e7ebd4576d3da4946c770ca17a04823d1e9b4 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 15 Dec 2023 13:25:37 +0100 Subject: [PATCH 02/14] Add test for deduping --- node-src/tasks/upload.test.ts | 66 +++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index 713f31012..ae168a5e4 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -412,6 +412,72 @@ describe('uploadStorybook', () => { }); }); + describe('with file hashes', () => { + it('retrieves file upload locations and uploads only returned targets', async () => { + const client = { runQuery: vi.fn() }; + client.runQuery.mockReturnValue({ + uploadBuild: { + info: { + targets: [ + { + contentType: 'text/html', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com/presigned?index.html', + formFields: {}, + }, + ], + }, + userErrors: [], + }, + }); + + createReadStreamMock.mockReturnValue({ pipe: vi.fn() } as any); + http.fetch.mockReturnValue({ ok: true }); + progress.mockReturnValue({ on: vi.fn() } as any); + + const fileInfo = { + lengths: [ + { knownAs: 'iframe.html', contentLength: 42 }, + { knownAs: 'index.html', contentLength: 42 }, + ], + hashes: { 'iframe.html': 'iframe', 'index.html': 'index' }, + paths: ['iframe.html', 'index.html'], + total: 84, + }; + const ctx = { + client, + env, + log, + http, + sourceDir: '/static/', + options: {}, + fileInfo, + announcedBuild: { id: '1' }, + } as any; + await uploadStorybook(ctx, {} as any); + + expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { + buildId: '1', + files: [ + { contentHash: 'iframe', contentLength: 42, filePath: 'iframe.html' }, + { contentHash: 'index', contentLength: 42, filePath: 'index.html' }, + ], + }); + expect(http.fetch).not.toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?iframe.html', + expect.anything(), + expect.anything() + ); + expect(http.fetch).toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?index.html', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), + expect.objectContaining({ retries: 0 }) + ); + expect(ctx.uploadedBytes).toBe(42); + expect(ctx.uploadedFiles).toBe(1); + }); + }); + describe('with zip', () => { it('retrieves the upload location, adds the files to an archive and uploads it', async () => { const client = { runQuery: vi.fn() }; From 55dbdb262b7e49e502f2a3ec4a3d14bd69266191 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 17 Dec 2023 19:33:51 +0100 Subject: [PATCH 03/14] Show number of skipped files --- node-src/ui/tasks/upload.stories.ts | 10 ++++++++++ node-src/ui/tasks/upload.ts | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/node-src/ui/tasks/upload.stories.ts b/node-src/ui/tasks/upload.stories.ts index cf66e6368..6547bb9d3 100644 --- a/node-src/ui/tasks/upload.stories.ts +++ b/node-src/ui/tasks/upload.stories.ts @@ -63,6 +63,16 @@ export const Success = () => startedAt: -54321, uploadedBytes: 1234567, uploadedFiles: 42, + fileInfo: { paths: { length: 42 } }, + } as any); + +export const SuccessSkippedFiles = () => + success({ + now: 0, + startedAt: -54321, + uploadedBytes: 1234567, + uploadedFiles: 42, + fileInfo: { paths: { length: 100 } }, } as any); export const SuccessNoFiles = () => success({} as any); diff --git a/node-src/ui/tasks/upload.ts b/node-src/ui/tasks/upload.ts index c9f169e27..8fc016689 100644 --- a/node-src/ui/tasks/upload.ts +++ b/node-src/ui/tasks/upload.ts @@ -102,10 +102,15 @@ export const uploading = ({ percentage }: { percentage: number }) => ({ export const success = (ctx: Context) => { const files = pluralize('file', ctx.uploadedFiles, true); const bytes = filesize(ctx.uploadedBytes || 0); + const skipped = + ctx.fileInfo.paths.length > ctx.uploadedFiles + ? `, skipped ${pluralize('file', ctx.fileInfo.paths.length - ctx.uploadedFiles, true)}` + : ''; + return { status: 'success', title: ctx.uploadedBytes ? `Publish complete in ${getDuration(ctx)}` : `Publish complete`, - output: ctx.uploadedBytes ? `Uploaded ${files} (${bytes})` : 'No new files to upload', + output: ctx.uploadedBytes ? `Uploaded ${files} (${bytes})${skipped}` : 'No new files to upload', }; }; From b80c2e0b2f71b6c4b47f3b13e8d56793c7027cac Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 17 Dec 2023 21:33:23 +0100 Subject: [PATCH 04/14] Fix story data --- node-src/ui/tasks/upload.stories.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node-src/ui/tasks/upload.stories.ts b/node-src/ui/tasks/upload.stories.ts index 6547bb9d3..b6264d9a2 100644 --- a/node-src/ui/tasks/upload.stories.ts +++ b/node-src/ui/tasks/upload.stories.ts @@ -75,6 +75,11 @@ export const SuccessSkippedFiles = () => fileInfo: { paths: { length: 100 } }, } as any); -export const SuccessNoFiles = () => success({} as any); +export const SuccessNoFiles = () => + success({ + uploadedBytes: 0, + uploadedFiles: 0, + fileInfo: { paths: { length: 100 } }, + } as any); export const Failed = () => failed({ path: 'main.9e3e453142da82719bf4.bundle.js' }); From 4ff08e3d94d81b9c577b16dda4c729a4f2e120c1 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 18 Dec 2023 14:48:50 +0100 Subject: [PATCH 05/14] Replace form-data with formdata-node and fix progress tracking --- node-src/lib/FileReaderBlob.ts | 20 ++++++++++++++++ node-src/lib/uploadFiles.ts | 15 ++++-------- node-src/lib/uploadZip.ts | 15 ++++-------- node-src/tasks/upload.test.ts | 42 +++++++++++++--------------------- package.json | 2 +- yarn.lock | 14 ++++-------- 6 files changed, 52 insertions(+), 56 deletions(-) create mode 100644 node-src/lib/FileReaderBlob.ts diff --git a/node-src/lib/FileReaderBlob.ts b/node-src/lib/FileReaderBlob.ts new file mode 100644 index 000000000..85129ae78 --- /dev/null +++ b/node-src/lib/FileReaderBlob.ts @@ -0,0 +1,20 @@ +import { ReadStream, createReadStream } from 'fs'; + +export class FileReaderBlob { + readStream: ReadStream; + size: number; + + constructor(filePath: string, contentLength: number, onProgress: (delta: number) => void) { + this.size = contentLength; + this.readStream = createReadStream(filePath); + this.readStream.on('data', (chunk: Buffer | string) => onProgress(chunk.length)); + } + + stream() { + return this.readStream; + } + + get [Symbol.toStringTag]() { + return 'Blob'; + } +} diff --git a/node-src/lib/uploadFiles.ts b/node-src/lib/uploadFiles.ts index b0e41e465..e30badf19 100644 --- a/node-src/lib/uploadFiles.ts +++ b/node-src/lib/uploadFiles.ts @@ -1,10 +1,9 @@ import retry from 'async-retry'; import { filesize } from 'filesize'; -import FormData from 'form-data'; -import { createReadStream } from 'fs'; +import { FormData } from 'formdata-node'; import pLimit from 'p-limit'; -import progress from 'progress-stream'; import { Context, FileDesc, TargetInfo } from '../types'; +import { FileReaderBlob } from './FileReaderBlob'; export async function uploadFiles( ctx: Context, @@ -28,19 +27,15 @@ export async function uploadFiles( return bail(signal.reason || new Error('Aborted')); } - const progressStream = progress(); - - progressStream.on('progress', ({ delta }) => { - fileProgress += delta; // We upload multiple files so we only care about the delta + const blob = new FileReaderBlob(localPath, contentLength, (delta) => { + fileProgress += delta; totalProgress += delta; onProgress?.(totalProgress); }); const formData = new FormData(); Object.entries(formFields).forEach(([k, v]) => formData.append(k, v)); - formData.append('file', createReadStream(localPath).pipe(progressStream), { - knownLength: contentLength, - }); + formData.append('file', blob); const res = await ctx.http.fetch( formAction, diff --git a/node-src/lib/uploadZip.ts b/node-src/lib/uploadZip.ts index 9aff4b712..a8778d139 100644 --- a/node-src/lib/uploadZip.ts +++ b/node-src/lib/uploadZip.ts @@ -1,10 +1,9 @@ import retry from 'async-retry'; import { filesize } from 'filesize'; -import FormData from 'form-data'; -import { createReadStream } from 'fs'; +import { FormData } from 'formdata-node'; import { Response } from 'node-fetch'; -import progress from 'progress-stream'; import { Context, TargetInfo } from '../types'; +import { FileReaderBlob } from './FileReaderBlob'; // A sentinel file is created by a zip-unpack lambda within the Chromatic infrastructure once the // uploaded zip is fully extracted. The contents of this file will consist of 'OK' if the process @@ -28,18 +27,14 @@ export async function uploadZip( return bail(signal.reason || new Error('Aborted')); } - const progressStream = progress(); - - progressStream.on('progress', ({ delta }) => { + const blob = new FileReaderBlob(localPath, contentLength, (delta) => { totalProgress += delta; - onProgress(totalProgress); + onProgress?.(totalProgress); }); const formData = new FormData(); Object.entries(formFields).forEach(([k, v]) => formData.append(k, v)); - formData.append('file', createReadStream(localPath).pipe(progressStream), { - knownLength: contentLength, - }); + formData.append('file', blob); const res = await ctx.http.fetch( formAction, diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index ae168a5e4..792ddd66f 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -1,6 +1,5 @@ -import FormData from 'form-data'; +import { FormData } from 'formdata-node'; import { createReadStream, readdirSync, readFileSync, statSync } from 'fs'; -import progressStream from 'progress-stream'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { default as compress } from '../lib/compress'; @@ -11,13 +10,21 @@ import { calculateFileHashes, validateFiles, traceChangedFiles, uploadStorybook vi.mock('form-data'); vi.mock('fs'); -vi.mock('progress-stream'); vi.mock('../lib/compress'); vi.mock('../lib/getDependentStoryFiles'); vi.mock('../lib/findChangedDependencies'); vi.mock('../lib/findChangedPackageFiles'); vi.mock('./read-stats-file'); +vi.mock('../lib/FileReaderBlob', () => ({ + FileReaderBlob: class { + constructor(path: string, length: number, onProgress: (delta: number) => void) { + onProgress(length / 2); + onProgress(length / 2); + } + }, +})); + vi.mock('../lib/getFileHashes', () => ({ getFileHashes: (files: string[]) => Promise.resolve(Object.fromEntries(files.map((f) => [f, 'hash']))), @@ -31,7 +38,6 @@ const createReadStreamMock = vi.mocked(createReadStream); const readdirSyncMock = vi.mocked(readdirSync); const readFileSyncMock = vi.mocked(readFileSync); const statSyncMock = vi.mocked(statSync); -const progress = vi.mocked(progressStream); const env = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; const log = { info: vi.fn(), warn: vi.fn(), debug: vi.fn() }; @@ -286,7 +292,6 @@ describe('uploadStorybook', () => { createReadStreamMock.mockReturnValue({ pipe: vi.fn() } as any); http.fetch.mockReturnValue({ ok: true }); - progress.mockReturnValue({ on: vi.fn() } as any); const fileInfo = { lengths: [ @@ -318,18 +323,18 @@ describe('uploadStorybook', () => { expect(http.fetch).toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?iframe.html', expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), - expect.objectContaining({ retries: 0 }) + { retries: 0 } ); expect(http.fetch).toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?index.html', expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), - expect.objectContaining({ retries: 0 }) + { retries: 0 } ); expect(ctx.uploadedBytes).toBe(84); expect(ctx.uploadedFiles).toBe(2); }); - it.skip('calls experimental_onTaskProgress with progress', async () => { + it('calls experimental_onTaskProgress with progress', async () => { const client = { runQuery: vi.fn() }; client.runQuery.mockReturnValue({ uploadBuild: { @@ -354,20 +359,7 @@ describe('uploadStorybook', () => { }); createReadStreamMock.mockReturnValue({ pipe: vi.fn((x) => x) } as any); - progress.mockImplementation((() => { - let progressCb; - return { - on: vi.fn((name, cb) => { - progressCb = cb; - }), - sendProgress: (delta: number) => progressCb({ delta }), - }; - }) as any); - http.fetch.mockReset().mockImplementation(async (url, { body }) => { - // How to update progress? - console.log(body); - return { ok: true }; - }); + http.fetch.mockReturnValue({ ok: true }); const fileInfo = { lengths: [ @@ -433,7 +425,6 @@ describe('uploadStorybook', () => { createReadStreamMock.mockReturnValue({ pipe: vi.fn() } as any); http.fetch.mockReturnValue({ ok: true }); - progress.mockReturnValue({ on: vi.fn() } as any); const fileInfo = { lengths: [ @@ -471,7 +462,7 @@ describe('uploadStorybook', () => { expect(http.fetch).toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?index.html', expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), - expect.objectContaining({ retries: 0 }) + { retries: 0 } ); expect(ctx.uploadedBytes).toBe(42); expect(ctx.uploadedFiles).toBe(1); @@ -513,7 +504,6 @@ describe('uploadStorybook', () => { makeZipFile.mockReturnValue(Promise.resolve({ path: 'storybook.zip', size: 80 })); createReadStreamMock.mockReturnValue({ pipe: vi.fn() } as any); http.fetch.mockReturnValue({ ok: true, text: () => Promise.resolve('OK') }); - progress.mockReturnValue({ on: vi.fn() } as any); const fileInfo = { lengths: [ @@ -546,7 +536,7 @@ describe('uploadStorybook', () => { expect(http.fetch).toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?storybook.zip', expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), - expect.objectContaining({ retries: 0 }) + { retries: 0 } ); expect(http.fetch).not.toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?iframe.html', diff --git a/package.json b/package.json index cc1902607..568b1e9f2 100644 --- a/package.json +++ b/package.json @@ -149,7 +149,7 @@ "execa": "^7.2.0", "fake-tag": "^2.0.0", "filesize": "^10.1.0", - "form-data": "^4.0.0", + "formdata-node": "^6.0.3", "fs-extra": "^10.0.0", "https-proxy-agent": "^7.0.2", "husky": "^7.0.0", diff --git a/yarn.lock b/yarn.lock index 54b6b1dea..d8bfa55d8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7893,20 +7893,16 @@ form-data@^3.0.0: combined-stream "^1.0.8" mime-types "^2.1.12" -form-data@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/form-data/-/form-data-4.0.0.tgz#93919daeaf361ee529584b9b31664dc12c9fa452" - integrity sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww== - dependencies: - asynckit "^0.4.0" - combined-stream "^1.0.8" - mime-types "^2.1.12" - format@^0.2.0: version "0.2.2" resolved "https://registry.yarnpkg.com/format/-/format-0.2.2.tgz#d6170107e9efdc4ed30c9dc39016df942b5cb58b" integrity sha1-1hcBB+nv3E7TDJ3DkBbflCtctYs= +formdata-node@^6.0.3: + version "6.0.3" + resolved "https://registry.yarnpkg.com/formdata-node/-/formdata-node-6.0.3.tgz#48f8e2206ae2befded82af621ef015f08168dc6d" + integrity sha512-8e1++BCiTzUno9v5IZ2J6bv4RU+3UKDmqWUQD0MIMVCd9AdhWkO1gw57oo1mNEX1dMq2EGI+FbWz4B92pscSQg== + formdata-polyfill@^4.0.10: version "4.0.10" resolved "https://registry.yarnpkg.com/formdata-polyfill/-/formdata-polyfill-4.0.10.tgz#24807c31c9d402e002ab3d8c720144ceb8848423" From 04988850062de45d8d4074b190b7aedd07fe0291 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 18 Dec 2023 14:57:40 +0100 Subject: [PATCH 06/14] Drop unused dependencies --- package.json | 2 -- yarn.lock | 22 +--------------------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/package.json b/package.json index 568b1e9f2..a3d1327a6 100644 --- a/package.json +++ b/package.json @@ -124,7 +124,6 @@ "@types/listr": "^0.14.4", "@types/node": "18.x", "@types/picomatch": "^2.3.0", - "@types/progress-stream": "^2.0.2", "@types/semver": "^7.3.9", "@typescript-eslint/eslint-plugin": "^6.8.0", "@typescript-eslint/parser": "^6.8.0", @@ -170,7 +169,6 @@ "pkg-up": "^3.1.0", "pluralize": "^8.0.0", "prettier": "^2.3.2", - "progress-stream": "^2.0.0", "prop-types": "^15.7.2", "react": "^17.0.2", "react-dom": "^17.0.2", diff --git a/yarn.lock b/yarn.lock index d8bfa55d8..d2dd8ecfe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3488,13 +3488,6 @@ resolved "https://registry.yarnpkg.com/@types/pretty-hrtime/-/pretty-hrtime-1.0.1.tgz#72a26101dc567b0d68fd956cf42314556e42d601" integrity sha512-VjID5MJb1eGKthz2qUerWT8+R4b9N+CHvGCzg9fn4kWZgaF9AhdYikQio3R7wV8YY1NsQKPaCwKz1Yff+aHNUQ== -"@types/progress-stream@^2.0.2": - version "2.0.2" - resolved "https://registry.yarnpkg.com/@types/progress-stream/-/progress-stream-2.0.2.tgz#443afb2c29cfde0e6240805364b7715bc5bd03a8" - integrity sha512-u9N40mYX/Nx/Pmt847+G2N72s5QL2jwgXrVKCIcxgOdMBdIzY+e/m3m1gQBNPmgvQQBO79EisLAcVJ/p0qKuvA== - dependencies: - "@types/node" "*" - "@types/prop-types@*": version "15.7.4" resolved "https://registry.yarnpkg.com/@types/prop-types/-/prop-types-15.7.4.tgz#fcf7205c25dff795ee79af1e30da2c9790808f11" @@ -12198,14 +12191,6 @@ process@^0.11.10: resolved "https://registry.yarnpkg.com/process/-/process-0.11.10.tgz#7332300e840161bda3e69a1d1d91a7d4bc16f182" integrity sha1-czIwDoQBYb2j5podHZGn1LwW8YI= -progress-stream@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/progress-stream/-/progress-stream-2.0.0.tgz#fac63a0b3d11deacbb0969abcc93b214bce19ed5" - integrity sha1-+sY6Cz0R3qy7CWmrzJOyFLzhntU= - dependencies: - speedometer "~1.0.0" - through2 "~2.0.3" - progress@^2.0.0: version "2.0.3" resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.3.tgz#7e8cf8d8f5b8f239c1bc68beb4eb78567d572ef8" @@ -13860,11 +13845,6 @@ spdx-license-ids@^3.0.0: resolved "https://registry.yarnpkg.com/spdx-license-ids/-/spdx-license-ids-3.0.10.tgz#0d9becccde7003d6c658d487dd48a32f0bf3014b" integrity sha512-oie3/+gKf7QtpitB0LYLETe+k8SifzsX4KixvpOsbI6S0kRiRQ5MKOio8eMSAKQ17N06+wdEOXRiId+zOxo0hA== -speedometer@~1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/speedometer/-/speedometer-1.0.0.tgz#cd671cb06752c22bca3370e2f334440be4fc62e2" - integrity sha1-zWccsGdSwivKM3Di8zREC+T8YuI= - split-string@^3.0.1, split-string@^3.0.2: version "3.1.0" resolved "https://registry.yarnpkg.com/split-string/-/split-string-3.1.0.tgz#7cb09dda3a86585705c64b39a6466038682e8fe2" @@ -14519,7 +14499,7 @@ thenify-all@^1.0.0: dependencies: any-promise "^1.0.0" -through2@^2.0.0, through2@~2.0.3: +through2@^2.0.0: version "2.0.5" resolved "https://registry.yarnpkg.com/through2/-/through2-2.0.5.tgz#01c1e39eb31d07cb7d03a96a70823260b23132cd" integrity sha512-/mrRod8xqpA+IHSLyGCQ2s8SPHiCDEeQJSep1jqLYeEUClOFG2Qsh+4FU6G9VeqpZnGW/Su8LQGc4YKni5rYSQ== From 96834e655e6fe0aed0b660de9dc38132fc085711 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 18 Dec 2023 14:57:49 +0100 Subject: [PATCH 07/14] Fix tests --- node-src/index.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/node-src/index.test.ts b/node-src/index.test.ts index 6f29249a7..072a9c5ef 100644 --- a/node-src/index.test.ts +++ b/node-src/index.test.ts @@ -469,6 +469,7 @@ it('calls out to npm build script passed and uploads files', async () => { expect.any(Object), [ { + contentHash: 'hash', contentLength: 42, contentType: 'text/html', fileKey: '', @@ -479,6 +480,7 @@ it('calls out to npm build script passed and uploads files', async () => { targetPath: 'iframe.html', }, { + contentHash: 'hash', contentLength: 42, contentType: 'text/html', fileKey: '', @@ -502,6 +504,7 @@ it('skips building and uploads directly with storybook-build-dir', async () => { expect.any(Object), [ { + contentHash: 'hash', contentLength: 42, contentType: 'text/html', fileKey: '', @@ -512,6 +515,7 @@ it('skips building and uploads directly with storybook-build-dir', async () => { targetPath: 'iframe.html', }, { + contentHash: 'hash', contentLength: 42, contentType: 'text/html', fileKey: '', From c99cfb45f18a6028c543f6372de91b5962dceeff Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 19 Dec 2023 11:40:31 +0100 Subject: [PATCH 08/14] Retrieve sentinelUrls from uploadBuild and wait for all of them before finishing upload task --- node-src/lib/upload.ts | 20 +++++++------ node-src/lib/uploadZip.ts | 45 +---------------------------- node-src/lib/waitForSentinel.ts | 45 +++++++++++++++++++++++++++++ node-src/tasks/upload.ts | 13 ++++++++- node-src/types.ts | 1 + node-src/ui/tasks/upload.stories.ts | 3 ++ node-src/ui/tasks/upload.ts | 6 ++++ 7 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 node-src/lib/waitForSentinel.ts diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index e9d354c93..1e43e3260 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -1,7 +1,7 @@ import makeZipFile from './compress'; -import { Context, FileDesc, TargetInfo } from '../types'; -import { uploadZip, waitForUnpack } from './uploadZip'; +import { uploadZip } from './uploadZip'; import { uploadFiles } from './uploadFiles'; +import { Context, FileDesc, TargetInfo } from '../types'; import { maxFileCountExceeded } from '../ui/messages/errors/maxFileCountExceeded'; import { maxFileSizeExceeded } from '../ui/messages/errors/maxFileSizeExceeded'; @@ -9,6 +9,7 @@ const UploadBuildMutation = ` mutation UploadBuildMutation($buildId: ObjID!, $files: [FileUploadInput!]!, $zip: Boolean) { uploadBuild(buildId: $buildId, files: $files, zip: $zip) { info { + sentinelUrls targets { contentType fileKey @@ -22,7 +23,6 @@ const UploadBuildMutation = ` filePath formAction formFields - sentinelUrl } } userErrors { @@ -46,8 +46,9 @@ const UploadBuildMutation = ` interface UploadBuildMutationResult { uploadBuild: { info?: { + sentinelUrls: string[]; targets: TargetInfo[]; - zipTarget?: TargetInfo & { sentinelUrl: string }; + zipTarget?: TargetInfo; }; userErrors: ( | { @@ -76,7 +77,7 @@ export async function uploadBuild( options: { onStart?: () => void; onProgress?: (progress: number, total: number) => void; - onComplete?: (uploadedBytes: number, uploadedFiles: number) => void; + onComplete?: (uploadedBytes: number, uploadedFiles: number, sentinelUrls: string[]) => void; onError?: (error: Error, path?: string) => void; } = {} ) { @@ -106,6 +107,8 @@ export async function uploadBuild( return options.onError?.(new Error('Upload rejected due to user error')); } + const { sentinelUrls } = uploadBuild.info; + const targets = uploadBuild.info.targets.map((target) => { const file = files.find((f) => f.targetPath === target.filePath); return { ...file, ...target }; @@ -113,7 +116,7 @@ export async function uploadBuild( if (!targets.length) { ctx.log.debug('No new files to upload, continuing'); - return options.onComplete?.(0, 0); + return options.onComplete?.(0, 0, sentinelUrls); } options.onStart?.(); @@ -127,8 +130,7 @@ export async function uploadBuild( const target = { ...uploadBuild.info.zipTarget, contentLength: size, localPath: path }; await uploadZip(ctx, target, (progress) => options.onProgress?.(progress, size)); - await waitForUnpack(ctx, target.sentinelUrl); - return options.onComplete?.(size, targets.length); + return options.onComplete?.(size, targets.length, sentinelUrls); } catch (err) { ctx.log.debug({ err }, 'Error uploading zip, falling back to uploading individual files'); } @@ -136,7 +138,7 @@ export async function uploadBuild( try { await uploadFiles(ctx, targets, (progress) => options.onProgress?.(progress, total)); - return options.onComplete?.(total, targets.length); + return options.onComplete?.(total, targets.length, sentinelUrls); } catch (e) { return options.onError?.(e, files.some((f) => f.localPath === e.message) && e.message); } diff --git a/node-src/lib/uploadZip.ts b/node-src/lib/uploadZip.ts index a8778d139..be1527238 100644 --- a/node-src/lib/uploadZip.ts +++ b/node-src/lib/uploadZip.ts @@ -1,18 +1,12 @@ import retry from 'async-retry'; import { filesize } from 'filesize'; import { FormData } from 'formdata-node'; -import { Response } from 'node-fetch'; import { Context, TargetInfo } from '../types'; import { FileReaderBlob } from './FileReaderBlob'; -// A sentinel file is created by a zip-unpack lambda within the Chromatic infrastructure once the -// uploaded zip is fully extracted. The contents of this file will consist of 'OK' if the process -// completed successfully and 'ERROR' if an error occurred. -const SENTINEL_SUCCESS_VALUE = 'OK'; - export async function uploadZip( ctx: Context, - target: TargetInfo & { contentLength: number; localPath: string; sentinelUrl: string }, + target: TargetInfo & { contentLength: number; localPath: string }, onProgress: (progress: number) => void ) { const { experimental_abortSignal: signal } = ctx.options; @@ -58,40 +52,3 @@ export async function uploadZip( } ); } - -export async function waitForUnpack(ctx: Context, url: string) { - const { experimental_abortSignal: signal } = ctx.options; - - ctx.log.debug(`Waiting for zip unpack sentinel file to appear at '${url}'`); - - return retry( - async (bail) => { - if (signal?.aborted) { - return bail(signal.reason || new Error('Aborted')); - } - - let res: Response; - try { - res = await ctx.http.fetch(url, { signal }, { retries: 0, noLogErrorBody: true }); - } catch (e) { - const { response = {} } = e; - if (response.status === 403) { - return bail(new Error('Provided signature expired.')); - } - throw new Error('Sentinel file not present.'); - } - - const result = await res.text(); - if (result !== SENTINEL_SUCCESS_VALUE) { - return bail(new Error('Zip file failed to unpack remotely.')); - } else { - ctx.log.debug(`Sentinel file present, continuing.`); - } - }, - { - retries: 185, // 3 minutes and some change (matches the lambda timeout with some extra buffer) - minTimeout: 1000, - maxTimeout: 1000, - } - ); -} diff --git a/node-src/lib/waitForSentinel.ts b/node-src/lib/waitForSentinel.ts new file mode 100644 index 000000000..2c7df6ca1 --- /dev/null +++ b/node-src/lib/waitForSentinel.ts @@ -0,0 +1,45 @@ +import retry from 'async-retry'; +import { Response } from 'node-fetch'; +import { Context } from '../types'; + +// A sentinel file is created by a zip-unpack lambda within the Chromatic infrastructure once the +// uploaded zip is fully extracted. The contents of this file will consist of 'OK' if the process +// completed successfully and 'ERROR' if an error occurred. +const SENTINEL_SUCCESS_VALUE = 'OK'; + +export async function waitForSentinel(ctx: Context, url: string) { + const { experimental_abortSignal: signal } = ctx.options; + + ctx.log.debug(`Waiting for sentinel file to appear at ${url}`); + + return retry( + async (bail) => { + if (signal?.aborted) { + return bail(signal.reason || new Error('Aborted')); + } + + let res: Response; + try { + res = await ctx.http.fetch(url, { signal }, { retries: 0, noLogErrorBody: true }); + } catch (e) { + const { response = {} } = e; + if (response.status === 403) { + return bail(new Error('Provided signature expired.')); + } + throw new Error('Sentinel file not present.'); + } + + const result = await res.text(); + if (result !== SENTINEL_SUCCESS_VALUE) { + ctx.log.debug(`Sentinel file not OK, got ${result}`); + return bail(new Error('Sentinel file error.')); + } + ctx.log.debug(`Sentinel file OK.`); + }, + { + retries: 185, // 3 minutes and some change (matches the lambda timeout with some extra buffer) + minTimeout: 1000, + maxTimeout: 1000, + } + ); +} diff --git a/node-src/tasks/upload.ts b/node-src/tasks/upload.ts index cacb17729..c1271c476 100644 --- a/node-src/tasks/upload.ts +++ b/node-src/tasks/upload.ts @@ -21,6 +21,7 @@ import { uploading, success, hashing, + finalizing, } from '../ui/tasks/upload'; import { Context, FileDesc, Task } from '../types'; import { readStatsFile } from './read-stats-file'; @@ -29,6 +30,7 @@ import { findChangedPackageFiles } from '../lib/findChangedPackageFiles'; import { findChangedDependencies } from '../lib/findChangedDependencies'; import { uploadBuild } from '../lib/upload'; import { getFileHashes } from '../lib/getFileHashes'; +import { waitForSentinel } from '../lib/waitForSentinel'; interface PathSpec { pathname: string; @@ -225,7 +227,8 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { // Avoid spamming the logs with progress updates in non-interactive mode ctx.options.interactive ? 100 : ctx.env.CHROMATIC_OUTPUT_INTERVAL ), - onComplete: (uploadedBytes: number, uploadedFiles: number) => { + onComplete: (uploadedBytes: number, uploadedFiles: number, sentinelUrls: string[]) => { + ctx.sentinelUrls = sentinelUrls; ctx.uploadedBytes = uploadedBytes; ctx.uploadedFiles = uploadedFiles; }, @@ -235,6 +238,13 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { }); }; +export const waitForSentinels = async (ctx: Context, task: Task) => { + if (ctx.skip || !ctx.sentinelUrls?.length) return; + transitionTo(finalizing)(ctx, task); + + await Promise.all(ctx.sentinelUrls.map((url) => waitForSentinel(ctx, url))); +}; + export default createTask({ name: 'upload', title: initial.title, @@ -249,6 +259,7 @@ export default createTask({ traceChangedFiles, calculateFileHashes, uploadStorybook, + waitForSentinels, transitionTo(success, true), ], }); diff --git a/node-src/types.ts b/node-src/types.ts index 4eee98335..7b3d6b285 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -302,6 +302,7 @@ export interface Context { }[]; total: number; }; + sentinelUrls?: string[]; uploadedBytes?: number; uploadedFiles?: number; turboSnap?: Partial<{ diff --git a/node-src/ui/tasks/upload.stories.ts b/node-src/ui/tasks/upload.stories.ts index b6264d9a2..44a8da5d8 100644 --- a/node-src/ui/tasks/upload.stories.ts +++ b/node-src/ui/tasks/upload.stories.ts @@ -3,6 +3,7 @@ import { bailed, dryRun, failed, + finalizing, hashing, initial, invalid, @@ -57,6 +58,8 @@ export const Starting = () => starting(); export const Uploading = () => uploading({ percentage: 42 }); +export const Finalizing = () => finalizing(); + export const Success = () => success({ now: 0, diff --git a/node-src/ui/tasks/upload.ts b/node-src/ui/tasks/upload.ts index 8fc016689..64c6c4ece 100644 --- a/node-src/ui/tasks/upload.ts +++ b/node-src/ui/tasks/upload.ts @@ -99,6 +99,12 @@ export const uploading = ({ percentage }: { percentage: number }) => ({ output: `${progressBar(percentage)} ${percentage}%`, }); +export const finalizing = () => ({ + status: 'pending', + title: 'Publishing your built Storybook', + output: `Finalizing upload`, +}); + export const success = (ctx: Context) => { const files = pluralize('file', ctx.uploadedFiles, true); const bytes = filesize(ctx.uploadedBytes || 0); From 7cb18284705facb87e7282efc1372bc204fe7d56 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Jan 2024 21:04:40 +0100 Subject: [PATCH 09/14] Revert "Revert "Replace `getUploadUrls` with `uploadBuild` mutation"" --- node-src/index.test.ts | 95 ++++--- node-src/index.ts | 2 +- node-src/lib/compress.ts | 2 +- node-src/lib/upload.ts | 233 ++++++++++++------ node-src/lib/uploadFiles.ts | 43 ++-- node-src/lib/uploadMetadataFiles.ts | 13 +- node-src/lib/uploadZip.ts | 37 ++- node-src/tasks/report.ts | 7 +- node-src/tasks/storybookInfo.test.ts | 4 +- node-src/tasks/upload.test.ts | 174 +++++++------ node-src/tasks/upload.ts | 39 +-- node-src/tasks/verify.test.ts | 17 +- node-src/tasks/verify.ts | 19 +- node-src/types.ts | 16 +- .../errors/brokenStorybook.stories.ts | 4 +- .../ui/messages/errors/brokenStorybook.ts | 5 +- .../ui/messages/errors/fatalError.stories.ts | 4 +- node-src/ui/messages/errors/fatalError.ts | 14 +- .../errors/maxFileCountExceeded.stories.ts | 11 + .../messages/errors/maxFileCountExceeded.ts | 19 ++ .../errors/maxFileSizeExceeded.stories.ts | 8 + .../ui/messages/errors/maxFileSizeExceeded.ts | 19 ++ .../info/storybookPublished.stories.ts | 7 +- .../ui/messages/info/storybookPublished.ts | 16 +- node-src/ui/tasks/upload.stories.ts | 13 +- node-src/ui/tasks/upload.ts | 17 +- package.json | 4 + yarn.lock | 9 + 28 files changed, 535 insertions(+), 316 deletions(-) create mode 100644 node-src/ui/messages/errors/maxFileCountExceeded.stories.ts create mode 100644 node-src/ui/messages/errors/maxFileCountExceeded.ts create mode 100644 node-src/ui/messages/errors/maxFileSizeExceeded.stories.ts create mode 100644 node-src/ui/messages/errors/maxFileSizeExceeded.ts diff --git a/node-src/index.test.ts b/node-src/index.test.ts index 6698b0778..6f29249a7 100644 --- a/node-src/index.test.ts +++ b/node-src/index.test.ts @@ -98,14 +98,16 @@ vi.mock('node-fetch', () => ({ } if (query?.match('PublishBuildMutation')) { - if (variables.input.isolatorUrl.startsWith('http://throw-an-error')) { - throw new Error('fetch error'); - } - publishedBuild = { id: variables.id, ...variables.input }; + publishedBuild = { + id: variables.id, + ...variables.input, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', + }; return { data: { publishBuild: { status: 'PUBLISHED', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', }, }, }; @@ -132,8 +134,8 @@ vi.mock('node-fetch', () => ({ status: 'IN_PROGRESS', specCount: 1, componentCount: 1, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', webUrl: 'http://test.com', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', ...mockBuildFeatures, app: { account: { @@ -193,7 +195,8 @@ vi.mock('node-fetch', () => ({ }; } - if (query?.match('GetUploadUrlsMutation')) { + if (query?.match('UploadBuildMutation') || query?.match('UploadMetadataMutation')) { + const key = query?.match('UploadBuildMutation') ? 'uploadBuild' : 'uploadMetadata'; const contentTypes = { html: 'text/html', js: 'text/javascript', @@ -202,13 +205,17 @@ vi.mock('node-fetch', () => ({ }; return { data: { - getUploadUrls: { - domain: 'https://chromatic.com', - urls: variables.paths.map((path: string) => ({ - path, - url: `https://cdn.example.com/${path}`, - contentType: contentTypes[path.split('.').at(-1)], - })), + [key]: { + info: { + targets: variables.files.map(({ filePath }) => ({ + contentType: contentTypes[filePath.split('.').at(-1)], + fileKey: '', + filePath, + formAction: 'https://s3.amazonaws.com', + formFields: {}, + })), + }, + userErrors: [], }, }, }; @@ -405,7 +412,7 @@ it('runs in simple situations', async () => { storybookViewLayer: 'viewLayer', committerEmail: 'test@test.com', committerName: 'tester', - isolatorUrl: `https://chromatic.com/iframe.html`, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', }); }); @@ -462,20 +469,24 @@ it('calls out to npm build script passed and uploads files', async () => { expect.any(Object), [ { - contentHash: 'hash', contentLength: 42, contentType: 'text/html', + fileKey: '', + filePath: 'iframe.html', + formAction: 'https://s3.amazonaws.com', + formFields: {}, localPath: expect.stringMatching(/\/iframe\.html$/), targetPath: 'iframe.html', - targetUrl: 'https://cdn.example.com/iframe.html', }, { - contentHash: 'hash', contentLength: 42, contentType: 'text/html', + fileKey: '', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com', + formFields: {}, localPath: expect.stringMatching(/\/index\.html$/), targetPath: 'index.html', - targetUrl: 'https://cdn.example.com/index.html', }, ], expect.any(Function) @@ -491,20 +502,24 @@ it('skips building and uploads directly with storybook-build-dir', async () => { expect.any(Object), [ { - contentHash: 'hash', contentLength: 42, contentType: 'text/html', + fileKey: '', + filePath: 'iframe.html', + formAction: 'https://s3.amazonaws.com', + formFields: {}, localPath: expect.stringMatching(/\/iframe\.html$/), targetPath: 'iframe.html', - targetUrl: 'https://cdn.example.com/iframe.html', }, { - contentHash: 'hash', contentLength: 42, contentType: 'text/html', + fileKey: '', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com', + formFields: {}, localPath: expect.stringMatching(/\/index\.html$/), targetPath: 'index.html', - targetUrl: 'https://cdn.example.com/index.html', }, ], expect.any(Function) @@ -699,32 +714,44 @@ it('should upload metadata files if --upload-metadata is passed', async () => { expect(upload.mock.calls.at(-1)[1]).toEqual( expect.arrayContaining([ { - localPath: '.storybook/main.js', - targetPath: '.chromatic/main.js', contentLength: 518, contentType: 'text/javascript', - targetUrl: 'https://cdn.example.com/.chromatic/main.js', + fileKey: '', + filePath: '.chromatic/main.js', + formAction: 'https://s3.amazonaws.com', + formFields: {}, + localPath: '.storybook/main.js', + targetPath: '.chromatic/main.js', }, { - localPath: 'storybook-out/preview-stats.trimmed.json', - targetPath: '.chromatic/preview-stats.trimmed.json', contentLength: 457, contentType: 'application/json', - targetUrl: 'https://cdn.example.com/.chromatic/preview-stats.trimmed.json', + fileKey: '', + filePath: '.chromatic/preview-stats.trimmed.json', + formAction: 'https://s3.amazonaws.com', + formFields: {}, + localPath: 'storybook-out/preview-stats.trimmed.json', + targetPath: '.chromatic/preview-stats.trimmed.json', }, { - localPath: '.storybook/preview.js', - targetPath: '.chromatic/preview.js', contentLength: 1338, contentType: 'text/javascript', - targetUrl: 'https://cdn.example.com/.chromatic/preview.js', + fileKey: '', + filePath: '.chromatic/preview.js', + formAction: 'https://s3.amazonaws.com', + formFields: {}, + localPath: '.storybook/preview.js', + targetPath: '.chromatic/preview.js', }, { - localPath: expect.any(String), - targetPath: '.chromatic/index.html', contentLength: expect.any(Number), contentType: 'text/html', - targetUrl: 'https://cdn.example.com/.chromatic/index.html', + fileKey: '', + filePath: '.chromatic/index.html', + formAction: 'https://s3.amazonaws.com', + formFields: {}, + localPath: expect.any(String), + targetPath: '.chromatic/index.html', }, ]) ); diff --git a/node-src/index.ts b/node-src/index.ts index 86a24359f..8b7e49193 100644 --- a/node-src/index.ts +++ b/node-src/index.ts @@ -120,7 +120,7 @@ export async function run({ code: ctx.exitCode, url: ctx.build?.webUrl, buildUrl: ctx.build?.webUrl, - storybookUrl: ctx.build?.cachedUrl?.replace(/iframe\.html.*$/, ''), + storybookUrl: ctx.build?.storybookUrl, specCount: ctx.build?.specCount, componentCount: ctx.build?.componentCount, testCount: ctx.build?.testCount, diff --git a/node-src/lib/compress.ts b/node-src/lib/compress.ts index b19d00d5c..312560034 100644 --- a/node-src/lib/compress.ts +++ b/node-src/lib/compress.ts @@ -24,7 +24,7 @@ export default async function makeZipFile(ctx: Context, files: FileDesc[]) { archive.pipe(sink); files.forEach(({ localPath, targetPath: name }) => { - ctx.log.debug({ name }, 'Adding file to zip archive'); + ctx.log.debug(`Adding to zip archive: ${name}`); archive.append(createReadStream(localPath), { name }); }); diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index c7e679e61..f5ad83571 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -1,112 +1,199 @@ import makeZipFile from './compress'; -import { Context, FileDesc, TargetedFile } from '../types'; +import { Context, FileDesc, TargetInfo } from '../types'; import { uploadZip, waitForUnpack } from './uploadZip'; import { uploadFiles } from './uploadFiles'; +import { maxFileCountExceeded } from '../ui/messages/errors/maxFileCountExceeded'; +import { maxFileSizeExceeded } from '../ui/messages/errors/maxFileSizeExceeded'; -const GetUploadUrlsMutation = ` - mutation GetUploadUrlsMutation($buildId: ObjID, $paths: [String!]!) { - getUploadUrls(buildId: $buildId, paths: $paths) { - domain - urls { - path - url - contentType +const UploadBuildMutation = ` + mutation UploadBuildMutation($buildId: ObjID!, $files: [FileUploadInput!]!, $zip: Boolean) { + uploadBuild(buildId: $buildId, files: $files, zip: $zip) { + info { + targets { + contentType + fileKey + filePath + formAction + formFields + } + zipTarget { + contentType + fileKey + filePath + formAction + formFields + sentinelUrl + } + } + userErrors { + __typename + ... on UserError { + message + } + ... on MaxFileCountExceededError { + maxFileCount + fileCount + } + ... on MaxFileSizeExceededError { + maxFileSize + filePaths + } } } } `; -interface GetUploadUrlsMutationResult { - getUploadUrls: { - domain: string; - urls: { - path: string; - url: string; - contentType: string; - }[]; - }; -} -const GetZipUploadUrlMutation = ` - mutation GetZipUploadUrlMutation($buildId: ObjID) { - getZipUploadUrl(buildId: $buildId) { - domain - url - sentinelUrl - } - } -`; -interface GetZipUploadUrlMutationResult { - getZipUploadUrl: { - domain: string; - url: string; - sentinelUrl: string; +interface UploadBuildMutationResult { + uploadBuild: { + info?: { + targets: TargetInfo[]; + zipTarget?: TargetInfo & { sentinelUrl: string }; + }; + userErrors: ( + | { + __typename: 'UserError'; + message: string; + } + | { + __typename: 'MaxFileCountExceededError'; + message: string; + maxFileCount: number; + fileCount: number; + } + | { + __typename: 'MaxFileSizeExceededError'; + message: string; + maxFileSize: number; + filePaths: string[]; + } + )[]; }; } -export async function uploadAsIndividualFiles( +export async function uploadBuild( ctx: Context, files: FileDesc[], options: { onStart?: () => void; onProgress?: (progress: number, total: number) => void; - onComplete?: (uploadedBytes: number, domain?: string) => void; + onComplete?: (uploadedBytes: number, uploadedFiles: number) => void; onError?: (error: Error, path?: string) => void; } = {} ) { - const { getUploadUrls } = await ctx.client.runQuery( - GetUploadUrlsMutation, - { buildId: ctx.announcedBuild.id, paths: files.map(({ targetPath }) => targetPath) } + const { uploadBuild } = await ctx.client.runQuery( + UploadBuildMutation, + { + buildId: ctx.announcedBuild.id, + files: files.map(({ contentLength, targetPath }) => ({ + contentLength, + filePath: targetPath, + })), + zip: ctx.options.zip, + } ); - const { domain, urls } = getUploadUrls; - const targets = urls.map(({ path, url, contentType }) => { - const file = files.find((f) => f.targetPath === path); - return { ...file, contentType, targetUrl: url }; + + if (uploadBuild.userErrors.length) { + uploadBuild.userErrors.forEach((e) => { + if (e.__typename === 'MaxFileCountExceededError') { + ctx.log.error(maxFileCountExceeded(e)); + } else if (e.__typename === 'MaxFileSizeExceededError') { + ctx.log.error(maxFileSizeExceeded(e)); + } else { + ctx.log.error(e.message); + } + }); + return options.onError?.(new Error('Upload rejected due to user error')); + } + + const targets = uploadBuild.info.targets.map((target) => { + const file = files.find((f) => f.targetPath === target.filePath); + return { ...file, ...target }; }); - const total = targets.reduce((acc, { contentLength }) => acc + contentLength, 0); + + if (!targets.length) { + ctx.log.debug('No new files to upload, continuing'); + return options.onComplete?.(0, 0); + } options.onStart?.(); + const total = targets.reduce((acc, { contentLength }) => acc + contentLength, 0); + if (uploadBuild.info.zipTarget) { + try { + const { path, size } = await makeZipFile(ctx, targets); + const compressionRate = (total - size) / total; + ctx.log.debug(`Compression reduced upload size by ${Math.round(compressionRate * 100)}%`); + + const target = { ...uploadBuild.info.zipTarget, contentLength: size, localPath: path }; + await uploadZip(ctx, target, (progress) => options.onProgress?.(progress, size)); + await waitForUnpack(ctx, target.sentinelUrl); + return options.onComplete?.(size, targets.length); + } catch (err) { + ctx.log.debug({ err }, 'Error uploading zip, falling back to uploading individual files'); + } + } + try { await uploadFiles(ctx, targets, (progress) => options.onProgress?.(progress, total)); + return options.onComplete?.(total, targets.length); } catch (e) { return options.onError?.(e, files.some((f) => f.localPath === e.message) && e.message); } - - options.onComplete?.(total, domain); } -export async function uploadAsZipFile( - ctx: Context, - files: FileDesc[], - options: { - onStart?: () => void; - onProgress?: (progress: number, total: number) => void; - onComplete?: (uploadedBytes: number, domain?: string) => void; - onError?: (error: Error, path?: string) => void; - } = {} -) { - const originalSize = files.reduce((acc, { contentLength }) => acc + contentLength, 0); - const zipped = await makeZipFile(ctx, files); - const { path, size } = zipped; +const UploadMetadataMutation = ` + mutation UploadMetadataMutation($buildId: ObjID!, $files: [FileUploadInput!]!) { + uploadMetadata(buildId: $buildId, files: $files) { + info { + targets { + contentType + fileKey + filePath + formAction + formFields + } + } + userErrors { + ... on UserError { + message + } + } + } + } +`; - if (size > originalSize) throw new Error('Zip file is larger than individual files'); - ctx.log.debug(`Compression reduced upload size by ${originalSize - size} bytes`); +interface UploadMetadataMutationResult { + uploadMetadata: { + info?: { + targets: TargetInfo[]; + }; + userErrors: { + message: string; + }[]; + }; +} - const { getZipUploadUrl } = await ctx.client.runQuery( - GetZipUploadUrlMutation, - { buildId: ctx.announcedBuild.id } +export async function uploadMetadata(ctx: Context, files: FileDesc[]) { + const { uploadMetadata } = await ctx.client.runQuery( + UploadMetadataMutation, + { + buildId: ctx.announcedBuild.id, + files: files.map(({ contentLength, targetPath }) => ({ + contentLength, + filePath: targetPath, + })), + } ); - const { domain, url, sentinelUrl } = getZipUploadUrl; - options.onStart?.(); - - try { - await uploadZip(ctx, path, url, size, (progress) => options.onProgress?.(progress, size)); - } catch (e) { - return options.onError?.(e, path); + if (uploadMetadata.info) { + const targets = uploadMetadata.info.targets.map((target) => { + const file = files.find((f) => f.targetPath === target.filePath); + return { ...file, ...target }; + }); + await uploadFiles(ctx, targets); } - await waitForUnpack(ctx, sentinelUrl); - - options.onComplete?.(size, domain); + if (uploadMetadata.userErrors.length) { + uploadMetadata.userErrors.forEach((e) => ctx.log.warn(e.message)); + } } diff --git a/node-src/lib/uploadFiles.ts b/node-src/lib/uploadFiles.ts index 24dc4ddd8..b0e41e465 100644 --- a/node-src/lib/uploadFiles.ts +++ b/node-src/lib/uploadFiles.ts @@ -1,25 +1,25 @@ import retry from 'async-retry'; +import { filesize } from 'filesize'; +import FormData from 'form-data'; import { createReadStream } from 'fs'; import pLimit from 'p-limit'; import progress from 'progress-stream'; -import { Context, TargetedFile } from '../types'; +import { Context, FileDesc, TargetInfo } from '../types'; export async function uploadFiles( ctx: Context, - files: TargetedFile[], - onProgress: (progress: number) => void + targets: (FileDesc & TargetInfo)[], + onProgress?: (progress: number) => void ) { const { experimental_abortSignal: signal } = ctx.options; const limitConcurrency = pLimit(10); let totalProgress = 0; await Promise.all( - files.map(({ localPath, targetUrl, contentType, contentLength }) => { + targets.map(({ contentLength, filePath, formAction, formFields, localPath }) => { let fileProgress = 0; // The bytes uploaded for this this particular file - ctx.log.debug( - `Uploading ${contentLength} bytes of ${contentType} for '${localPath}' to '${targetUrl}'` - ); + ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)})`); return limitConcurrency(() => retry( @@ -33,37 +33,34 @@ export async function uploadFiles( progressStream.on('progress', ({ delta }) => { fileProgress += delta; // We upload multiple files so we only care about the delta totalProgress += delta; - onProgress(totalProgress); + onProgress?.(totalProgress); + }); + + const formData = new FormData(); + Object.entries(formFields).forEach(([k, v]) => formData.append(k, v)); + formData.append('file', createReadStream(localPath).pipe(progressStream), { + knownLength: contentLength, }); const res = await ctx.http.fetch( - targetUrl, - { - method: 'PUT', - body: createReadStream(localPath).pipe(progressStream), - headers: { - 'content-type': contentType, - 'content-length': contentLength.toString(), - 'cache-control': 'max-age=31536000', - }, - signal, - }, + formAction, + { body: formData, method: 'POST', signal }, { retries: 0 } // already retrying the whole operation ); if (!res.ok) { - ctx.log.debug(`Uploading '${localPath}' failed: %O`, res); + ctx.log.debug(`Uploading ${localPath} failed: %O`, res); throw new Error(localPath); } - ctx.log.debug(`Uploaded '${localPath}'.`); + ctx.log.debug(`Uploaded ${filePath} (${filesize(contentLength)})`); }, { retries: ctx.env.CHROMATIC_RETRIES, onRetry: (err: Error) => { totalProgress -= fileProgress; fileProgress = 0; - ctx.log.debug('Retrying upload %s, %O', targetUrl, err); - onProgress(totalProgress); + ctx.log.debug('Retrying upload for %s, %O', localPath, err); + onProgress?.(totalProgress); }, } ) diff --git a/node-src/lib/uploadMetadataFiles.ts b/node-src/lib/uploadMetadataFiles.ts index a2ba5876a..bbca940b3 100644 --- a/node-src/lib/uploadMetadataFiles.ts +++ b/node-src/lib/uploadMetadataFiles.ts @@ -7,8 +7,7 @@ import { Context, FileDesc } from '../types'; import metadataHtml from '../ui/html/metadata.html'; import uploadingMetadata from '../ui/messages/info/uploadingMetadata'; import { findStorybookConfigFile } from './getStorybookMetadata'; -import { uploadAsIndividualFiles } from './upload'; -import { baseStorybookUrl } from './utils'; +import { uploadMetadata } from './upload'; const fileSize = (path: string): Promise => new Promise((resolve) => stat(path, (err, stats) => resolve(err ? 0 : stats.size))); @@ -30,9 +29,9 @@ export async function uploadMetadataFiles(ctx: Context) { const files = await Promise.all( metadataFiles.map(async (localPath) => { - const targetPath = `.chromatic/${basename(localPath)}`; const contentLength = await fileSize(localPath); - return contentLength && { localPath, targetPath, contentLength }; + const targetPath = `.chromatic/${basename(localPath)}`; + return contentLength && { contentLength, localPath, targetPath }; }) ).then((files) => files @@ -49,14 +48,14 @@ export async function uploadMetadataFiles(ctx: Context) { const html = metadataHtml(ctx, files); writeFileSync(path, html); files.push({ + contentLength: html.length, localPath: path, targetPath: '.chromatic/index.html', - contentLength: html.length, }); - const directoryUrl = `${baseStorybookUrl(ctx.isolatorUrl)}/.chromatic/`; + const directoryUrl = `${ctx.build.storybookUrl}.chromatic/`; ctx.log.info(uploadingMetadata(directoryUrl, files)); - await uploadAsIndividualFiles(ctx, files); + await uploadMetadata(ctx, files); }); } diff --git a/node-src/lib/uploadZip.ts b/node-src/lib/uploadZip.ts index 068a828d7..9aff4b712 100644 --- a/node-src/lib/uploadZip.ts +++ b/node-src/lib/uploadZip.ts @@ -1,8 +1,10 @@ import retry from 'async-retry'; +import { filesize } from 'filesize'; +import FormData from 'form-data'; import { createReadStream } from 'fs'; import { Response } from 'node-fetch'; import progress from 'progress-stream'; -import { Context } from '../types'; +import { Context, TargetInfo } from '../types'; // A sentinel file is created by a zip-unpack lambda within the Chromatic infrastructure once the // uploaded zip is fully extracted. The contents of this file will consist of 'OK' if the process @@ -11,15 +13,14 @@ const SENTINEL_SUCCESS_VALUE = 'OK'; export async function uploadZip( ctx: Context, - path: string, - url: string, - contentLength: number, + target: TargetInfo & { contentLength: number; localPath: string; sentinelUrl: string }, onProgress: (progress: number) => void ) { const { experimental_abortSignal: signal } = ctx.options; + const { contentLength, filePath, formAction, formFields, localPath } = target; let totalProgress = 0; - ctx.log.debug(`Uploading ${contentLength} bytes for '${path}' to '${url}'`); + ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)})`); return retry( async (bail) => { @@ -34,31 +35,29 @@ export async function uploadZip( onProgress(totalProgress); }); + const formData = new FormData(); + Object.entries(formFields).forEach(([k, v]) => formData.append(k, v)); + formData.append('file', createReadStream(localPath).pipe(progressStream), { + knownLength: contentLength, + }); + const res = await ctx.http.fetch( - url, - { - method: 'PUT', - body: createReadStream(path).pipe(progressStream), - headers: { - 'content-type': 'application/zip', - 'content-length': contentLength.toString(), - }, - signal, - }, + formAction, + { body: formData, method: 'POST', signal }, { retries: 0 } // already retrying the whole operation ); if (!res.ok) { - ctx.log.debug(`Uploading '${path}' failed: %O`, res); - throw new Error(path); + ctx.log.debug(`Uploading ${localPath} failed: %O`, res); + throw new Error(localPath); } - ctx.log.debug(`Uploaded '${path}'.`); + ctx.log.debug(`Uploaded ${filePath} (${filesize(contentLength)})`); }, { retries: ctx.env.CHROMATIC_RETRIES, onRetry: (err: Error) => { totalProgress = 0; - ctx.log.debug('Retrying upload %s, %O', url, err); + ctx.log.debug('Retrying upload for %s, %O', localPath, err); onProgress(totalProgress); }, } diff --git a/node-src/tasks/report.ts b/node-src/tasks/report.ts index 299e5329f..c36b9d453 100644 --- a/node-src/tasks/report.ts +++ b/node-src/tasks/report.ts @@ -2,7 +2,6 @@ import reportBuilder from 'junit-report-builder'; import path from 'path'; import { createTask, transitionTo } from '../lib/tasks'; -import { baseStorybookUrl } from '../lib/utils'; import { Context } from '../types'; import wroteReport from '../ui/messages/info/wroteReport'; import { initial, pending, success } from '../ui/tasks/report'; @@ -13,8 +12,8 @@ const ReportQuery = ` build(number: $buildNumber) { number status(legacy: false) + storybookUrl webUrl - cachedUrl createdAt completedAt tests { @@ -44,8 +43,8 @@ interface ReportQueryResult { build: { number: number; status: string; + storybookUrl: string; webUrl: string; - cachedUrl: string; createdAt: number; completedAt: number; tests: { @@ -94,7 +93,7 @@ export const generateReport = async (ctx: Context) => { .property('buildNumber', build.number) .property('buildStatus', build.status) .property('buildUrl', build.webUrl) - .property('storybookUrl', baseStorybookUrl(build.cachedUrl)); + .property('storybookUrl', build.storybookUrl); build.tests.forEach(({ status, result, spec, parameters, mode }) => { const testSuffixName = mode.name || `[${parameters.viewport}px]`; diff --git a/node-src/tasks/storybookInfo.test.ts b/node-src/tasks/storybookInfo.test.ts index 0f00a09b2..b1bc59a14 100644 --- a/node-src/tasks/storybookInfo.test.ts +++ b/node-src/tasks/storybookInfo.test.ts @@ -7,8 +7,8 @@ vi.mock('../lib/getStorybookInfo'); const getStorybookInfo = vi.mocked(storybookInfo); -describe('startStorybook', () => { - it('starts the app and sets the isolatorUrl on context', async () => { +describe('storybookInfo', () => { + it('retrieves Storybook metadata and sets it on context', async () => { const storybook = { version: '1.0.0', viewLayer: 'react', addons: [] }; getStorybookInfo.mockResolvedValue(storybook); diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index 8835964ea..3bfb5817a 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -1,6 +1,7 @@ +import FormData from 'form-data'; import { createReadStream, readdirSync, readFileSync, statSync } from 'fs'; import progressStream from 'progress-stream'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { default as compress } from '../lib/compress'; import { getDependentStoryFiles as getDepStoryFiles } from '../lib/getDependentStoryFiles'; @@ -8,6 +9,7 @@ import { findChangedDependencies as findChangedDep } from '../lib/findChangedDep import { findChangedPackageFiles as findChangedPkg } from '../lib/findChangedPackageFiles'; import { calculateFileHashes, validateFiles, traceChangedFiles, uploadStorybook } from './upload'; +vi.mock('form-data'); vi.mock('fs'); vi.mock('progress-stream'); vi.mock('../lib/compress'); @@ -35,6 +37,10 @@ const env = { CHROMATIC_RETRIES: 2, CHROMATIC_OUTPUT_INTERVAL: 0 }; const log = { info: vi.fn(), warn: vi.fn(), debug: vi.fn() }; const http = { fetch: vi.fn() }; +afterEach(() => { + vi.restoreAllMocks(); +}); + describe('validateFiles', () => { it('sets fileInfo on context', async () => { readdirSyncMock.mockReturnValue(['iframe.html', 'index.html'] as any); @@ -254,23 +260,27 @@ describe('calculateFileHashes', () => { }); describe('uploadStorybook', () => { - it('retrieves the upload locations, puts the files there and sets the isolatorUrl on context', async () => { + it('retrieves the upload locations and uploads the files', async () => { const client = { runQuery: vi.fn() }; client.runQuery.mockReturnValue({ - getUploadUrls: { - domain: 'https://asdqwe.chromatic.com', - urls: [ - { - path: 'iframe.html', - url: 'https://asdqwe.chromatic.com/iframe.html', - contentType: 'text/html', - }, - { - path: 'index.html', - url: 'https://asdqwe.chromatic.com/index.html', - contentType: 'text/html', - }, - ], + uploadBuild: { + info: { + targets: [ + { + contentType: 'text/html', + filePath: 'iframe.html', + formAction: 'https://s3.amazonaws.com/presigned?iframe.html', + formFields: {}, + }, + { + contentType: 'text/html', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com/presigned?index.html', + formFields: {}, + }, + ], + }, + userErrors: [], }, }); @@ -298,55 +308,48 @@ describe('uploadStorybook', () => { } as any; await uploadStorybook(ctx, {} as any); - expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/GetUploadUrlsMutation/), { + expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { buildId: '1', - paths: ['iframe.html', 'index.html'], + files: [ + { contentLength: 42, filePath: 'iframe.html' }, + { contentLength: 42, filePath: 'index.html' }, + ], }); expect(http.fetch).toHaveBeenCalledWith( - 'https://asdqwe.chromatic.com/iframe.html', - expect.objectContaining({ - method: 'PUT', - headers: { - 'content-type': 'text/html', - 'content-length': '42', - 'cache-control': 'max-age=31536000', - }, - }), + 'https://s3.amazonaws.com/presigned?iframe.html', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), expect.objectContaining({ retries: 0 }) ); expect(http.fetch).toHaveBeenCalledWith( - 'https://asdqwe.chromatic.com/index.html', - expect.objectContaining({ - method: 'PUT', - headers: { - 'content-type': 'text/html', - 'content-length': '42', - 'cache-control': 'max-age=31536000', - }, - }), + 'https://s3.amazonaws.com/presigned?index.html', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), expect.objectContaining({ retries: 0 }) ); expect(ctx.uploadedBytes).toBe(84); - expect(ctx.isolatorUrl).toBe('https://asdqwe.chromatic.com/iframe.html'); + expect(ctx.uploadedFiles).toBe(2); }); - it('calls experimental_onTaskProgress with progress', async () => { + it.skip('calls experimental_onTaskProgress with progress', async () => { const client = { runQuery: vi.fn() }; client.runQuery.mockReturnValue({ - getUploadUrls: { - domain: 'https://asdqwe.chromatic.com', - urls: [ - { - path: 'iframe.html', - url: 'https://asdqwe.chromatic.com/iframe.html', - contentType: 'text/html', - }, - { - path: 'index.html', - url: 'https://asdqwe.chromatic.com/index.html', - contentType: 'text/html', - }, - ], + uploadBuild: { + info: { + targets: [ + { + contentType: 'text/html', + filePath: 'iframe.html', + formAction: 'https://s3.amazonaws.com/presigned?iframe.html', + formFields: {}, + }, + { + contentType: 'text/html', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com/presigned?index.html', + formFields: {}, + }, + ], + }, + userErrors: [], }, }); @@ -361,9 +364,8 @@ describe('uploadStorybook', () => { }; }) as any); http.fetch.mockReset().mockImplementation(async (url, { body }) => { - // body is just the mocked progress stream, as pipe returns it - body.sendProgress(21); - body.sendProgress(21); + // How to update progress? + console.log(body); return { ok: true }; }); @@ -414,10 +416,31 @@ describe('uploadStorybook', () => { it('retrieves the upload location, adds the files to an archive and uploads it', async () => { const client = { runQuery: vi.fn() }; client.runQuery.mockReturnValue({ - getZipUploadUrl: { - domain: 'https://asdqwe.chromatic.com', - url: 'https://asdqwe.chromatic.com/storybook.zip', - sentinelUrl: 'https://asdqwe.chromatic.com/upload.txt', + uploadBuild: { + info: { + targets: [ + { + contentType: 'text/html', + filePath: 'iframe.html', + formAction: 'https://s3.amazonaws.com/presigned?iframe.html', + formFields: {}, + }, + { + contentType: 'text/html', + filePath: 'index.html', + formAction: 'https://s3.amazonaws.com/presigned?index.html', + formFields: {}, + }, + ], + zipTarget: { + contentType: 'application/zip', + filePath: 'storybook.zip', + formAction: 'https://s3.amazonaws.com/presigned?storybook.zip', + formFields: {}, + sentinelUrl: 'https://asdqwe.chromatic.com/sentinel.txt', + }, + }, + userErrors: [], }, }); @@ -446,22 +469,31 @@ describe('uploadStorybook', () => { } as any; await uploadStorybook(ctx, {} as any); - expect(client.runQuery).toHaveBeenCalledWith( - expect.stringMatching(/GetZipUploadUrlMutation/), - { buildId: '1' } - ); + expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { + buildId: '1', + files: [ + { contentLength: 42, filePath: 'iframe.html' }, + { contentLength: 42, filePath: 'index.html' }, + ], + zip: true, + }); expect(http.fetch).toHaveBeenCalledWith( - 'https://asdqwe.chromatic.com/storybook.zip', - expect.objectContaining({ - method: 'PUT', - headers: { - 'content-type': 'application/zip', - 'content-length': '80', - }, - }), + 'https://s3.amazonaws.com/presigned?storybook.zip', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), expect.objectContaining({ retries: 0 }) ); + expect(http.fetch).not.toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?iframe.html', + expect.anything(), + expect.anything() + ); + expect(http.fetch).not.toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?iframe.html', + expect.anything(), + expect.anything() + ); expect(ctx.uploadedBytes).toBe(80); + expect(ctx.uploadedFiles).toBe(2); }); }); }); diff --git a/node-src/tasks/upload.ts b/node-src/tasks/upload.ts index 565b4877a..93903c3be 100644 --- a/node-src/tasks/upload.ts +++ b/node-src/tasks/upload.ts @@ -27,7 +27,7 @@ import { readStatsFile } from './read-stats-file'; import bailFile from '../ui/messages/warnings/bailFile'; import { findChangedPackageFiles } from '../lib/findChangedPackageFiles'; import { findChangedDependencies } from '../lib/findChangedDependencies'; -import { uploadAsIndividualFiles, uploadAsZipFile } from '../lib/upload'; +import { uploadBuild } from '../lib/upload'; import { getFileHashes } from '../lib/getFileHashes'; interface PathSpec { @@ -204,7 +204,15 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { if (ctx.skip) return; transitionTo(preparing)(ctx, task); - const options = { + const files = ctx.fileInfo.paths + .map((path) => ({ + contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === path).contentLength, + localPath: join(ctx.sourceDir, path), + targetPath: path, + })) + .filter((f) => f.contentLength); + + await uploadBuild(ctx, files, { onStart: () => (task.output = starting().output), onProgress: throttle( (progress, total) => { @@ -216,35 +224,14 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { // Avoid spamming the logs with progress updates in non-interactive mode ctx.options.interactive ? 100 : ctx.env.CHROMATIC_OUTPUT_INTERVAL ), - onComplete: (uploadedBytes: number, domain: string) => { + onComplete: (uploadedBytes: number, uploadedFiles: number) => { ctx.uploadedBytes = uploadedBytes; - ctx.isolatorUrl = new URL('/iframe.html', domain).toString(); + ctx.uploadedFiles = uploadedFiles; }, onError: (error: Error, path?: string) => { throw path === error.message ? new Error(failed({ path }).output) : error; }, - }; - - const files = ctx.fileInfo.paths.map((path) => ({ - localPath: join(ctx.sourceDir, path), - targetPath: path, - contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === path).contentLength, - ...(ctx.fileInfo.hashes && { contentHash: ctx.fileInfo.hashes[path] }), - })); - - if (ctx.options.zip) { - try { - await uploadAsZipFile(ctx, files, options); - } catch (err) { - ctx.log.debug( - { err }, - 'Error uploading zip file, falling back to uploading individual files' - ); - await uploadAsIndividualFiles(ctx, files, options); - } - } else { - await uploadAsIndividualFiles(ctx, files, options); - } + }); }; export default createTask({ diff --git a/node-src/tasks/verify.test.ts b/node-src/tasks/verify.test.ts index 2e2f9c3f9..fc0c34872 100644 --- a/node-src/tasks/verify.test.ts +++ b/node-src/tasks/verify.test.ts @@ -29,13 +29,7 @@ describe('publishBuild', () => { expect(client.runQuery).toHaveBeenCalledWith( expect.stringMatching(/PublishBuildMutation/), - { - input: { - cachedUrl: ctx.cachedUrl, - isolatorUrl: ctx.isolatorUrl, - turboSnapStatus: 'UNUSED', - }, - }, + { input: { turboSnapStatus: 'UNUSED' } }, { headers: { Authorization: `Bearer report-token` }, retries: 3 } ); expect(ctx.announcedBuild).toEqual({ ...announcedBuild, ...publishedBuild }); @@ -51,7 +45,6 @@ describe('verifyBuild', () => { git: { version: 'whatever', matchesBranch: () => false }, pkg: { version: '1.0.0' }, storybook: { version: '2.0.0', viewLayer: 'react', addons: [] }, - isolatorUrl: 'https://tunnel.chromaticqa.com/', announcedBuild: { number: 1, reportToken: 'report-token' }, }; @@ -109,7 +102,7 @@ describe('verifyBuild', () => { build: { number: 1, status: 'IN_PROGRESS', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', features: { uiTests: true, uiReview: false }, app: { account: {} }, wasLimited: true, @@ -130,7 +123,7 @@ describe('verifyBuild', () => { build: { number: 1, status: 'IN_PROGRESS', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', features: { uiTests: true, uiReview: false }, app: { account: { exceededThreshold: true } }, wasLimited: true, @@ -151,7 +144,7 @@ describe('verifyBuild', () => { build: { number: 1, status: 'IN_PROGRESS', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', features: { uiTests: true, uiReview: false }, app: { account: { paymentRequired: true } }, wasLimited: true, @@ -172,7 +165,7 @@ describe('verifyBuild', () => { build: { number: 1, status: 'IN_PROGRESS', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', features: { uiTests: false, uiReview: false }, app: { account: { paymentRequired: true } }, wasLimited: true, diff --git a/node-src/tasks/verify.ts b/node-src/tasks/verify.ts index f52581713..4bc4f4111 100644 --- a/node-src/tasks/verify.ts +++ b/node-src/tasks/verify.ts @@ -26,15 +26,19 @@ const PublishBuildMutation = ` publishBuild(id: $id, input: $input) { # no need for legacy:false on PublishedBuild.status status + storybookUrl } } `; interface PublishBuildMutationResult { - publishBuild: Context['announcedBuild']; + publishBuild: { + status: string; + storybookUrl: string; + }; } export const publishBuild = async (ctx: Context) => { - const { cachedUrl, isolatorUrl, turboSnap } = ctx; + const { turboSnap } = ctx; const { id, reportToken } = ctx.announcedBuild; const { replacementBuildIds } = ctx.git; const { onlyStoryNames, onlyStoryFiles = ctx.onlyStoryFiles } = ctx.options; @@ -51,8 +55,6 @@ export const publishBuild = async (ctx: Context) => { { id, input: { - cachedUrl, - isolatorUrl, ...(onlyStoryFiles && { onlyStoryFiles }), ...(onlyStoryNames && { onlyStoryNames: [].concat(onlyStoryNames) }), ...(replacementBuildIds && { replacementBuildIds }), @@ -66,12 +68,15 @@ export const publishBuild = async (ctx: Context) => { ); ctx.announcedBuild = { ...ctx.announcedBuild, ...publishedBuild }; + ctx.storybookUrl = publishedBuild.storybookUrl; // Queueing the extract may have failed if (publishedBuild.status === 'FAILED') { setExitCode(ctx, exitCodes.BUILD_FAILED, false); throw new Error(publishFailed().output); } + + ctx.log.info(storybookPublished(ctx)); }; const StartedBuildQuery = ` @@ -111,7 +116,6 @@ const VerifyBuildQuery = ` inheritedCaptureCount interactionTestFailuresCount webUrl - cachedUrl browsers { browser } @@ -161,7 +165,7 @@ interface VerifyBuildQueryResult { } export const verifyBuild = async (ctx: Context, task: Task) => { - const { client, isolatorUrl } = ctx; + const { client } = ctx; const { list, onlyStoryNames, onlyStoryFiles = ctx.onlyStoryFiles } = ctx.options; const { matchesBranch } = ctx.git; @@ -175,6 +179,7 @@ export const verifyBuild = async (ctx: Context, task: Task) => { } const waitForBuildToStart = async () => { + const { storybookUrl } = ctx; const { number, reportToken } = ctx.announcedBuild; const variables = { number }; const options = { headers: { Authorization: `Bearer ${reportToken}` } }; @@ -183,7 +188,7 @@ export const verifyBuild = async (ctx: Context, task: Task) => { app: { build }, } = await client.runQuery(StartedBuildQuery, variables, options); if (build.failureReason) { - ctx.log.warn(brokenStorybook({ ...build, isolatorUrl })); + ctx.log.warn(brokenStorybook({ ...build, storybookUrl })); setExitCode(ctx, exitCodes.STORYBOOK_BROKEN, true); throw new Error(publishFailed().output); } diff --git a/node-src/types.ts b/node-src/types.ts index 41de7720e..5014f3205 100644 --- a/node-src/types.ts +++ b/node-src/types.ts @@ -223,8 +223,7 @@ export interface Context { }; mainConfigFilePath?: string; }; - isolatorUrl: string; - cachedUrl: string; + storybookUrl?: string; announcedBuild: { id: string; number: number; @@ -241,7 +240,7 @@ export interface Context { number: number; status: string; webUrl: string; - cachedUrl: string; + storybookUrl: string; reportToken?: string; inheritedCaptureCount: number; actualCaptureCount: number; @@ -294,7 +293,7 @@ export interface Context { buildLogFile?: string; fileInfo?: { paths: string[]; - hashes: Record; + hashes?: Record; statsPath: string; lengths: { knownAs: string; @@ -304,6 +303,7 @@ export interface Context { total: number; }; uploadedBytes?: number; + uploadedFiles?: number; turboSnap?: Partial<{ unavailable?: boolean; rootPath: string; @@ -354,13 +354,15 @@ export interface Stats { } export interface FileDesc { - contentHash?: string; contentLength: number; localPath: string; targetPath: string; } -export interface TargetedFile extends FileDesc { +export interface TargetInfo { contentType: string; - targetUrl: string; + fileKey: string; + filePath: string; + formAction: string; + formFields: { [key: string]: string }; } diff --git a/node-src/ui/messages/errors/brokenStorybook.stories.ts b/node-src/ui/messages/errors/brokenStorybook.stories.ts index bfd65fa5c..44ccf67a1 100644 --- a/node-src/ui/messages/errors/brokenStorybook.stories.ts +++ b/node-src/ui/messages/errors/brokenStorybook.stories.ts @@ -15,6 +15,6 @@ ReferenceError: foo is not defined at https://61b0a4b8ebf0e344c2aa231c-nsoaxcirhi.capture.dev-chromatic.com/main.72ad6d7a.iframe.bundle.js:1:47 `; -const isolatorUrl = 'https://61b0a4b8ebf0e344c2aa231c-wdooytetbw.dev-chromatic.com'; +const storybookUrl = 'https://61b0a4b8ebf0e344c2aa231c-wdooytetbw.dev-chromatic.com/'; -export const BrokenStorybook = () => brokenStorybook({ failureReason, isolatorUrl }); +export const BrokenStorybook = () => brokenStorybook({ failureReason, storybookUrl }); diff --git a/node-src/ui/messages/errors/brokenStorybook.ts b/node-src/ui/messages/errors/brokenStorybook.ts index 94c5e77e6..b0c1fd3f2 100644 --- a/node-src/ui/messages/errors/brokenStorybook.ts +++ b/node-src/ui/messages/errors/brokenStorybook.ts @@ -1,16 +1,15 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; -import { baseStorybookUrl } from '../../../lib/utils'; import { error } from '../../components/icons'; import link from '../../components/link'; -export default ({ failureReason, isolatorUrl }) => +export default ({ failureReason, storybookUrl }) => `${dedent(chalk` ${error} {bold Failed to extract stories from your Storybook} This is usually a problem with your published Storybook, not with Chromatic. Build and open your Storybook locally and check the browser console for errors. - Visit your published Storybook at ${link(baseStorybookUrl(isolatorUrl))} + Visit your published Storybook at ${link(storybookUrl)} The following error was encountered while running your Storybook: `)}\n\n${failureReason.trim()}`; diff --git a/node-src/ui/messages/errors/fatalError.stories.ts b/node-src/ui/messages/errors/fatalError.stories.ts index 36faa7c4d..77a8a4f6a 100644 --- a/node-src/ui/messages/errors/fatalError.stories.ts +++ b/node-src/ui/messages/errors/fatalError.stories.ts @@ -47,10 +47,10 @@ const context = { build: { id: '5ec5069ae0d35e0022b6a9cc', number: 42, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', webUrl: 'https://www.chromatic.com/build?appId=5d67dc0374b2e300209c41e7&number=1400', }, - isolatorUrl: 'https://pfkaemtlit.tunnel.chromaticqa.com/iframe.html', - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', }; const stack = `Error: Oh no! diff --git a/node-src/ui/messages/errors/fatalError.ts b/node-src/ui/messages/errors/fatalError.ts index 218a14af1..00c4fd109 100644 --- a/node-src/ui/messages/errors/fatalError.ts +++ b/node-src/ui/messages/errors/fatalError.ts @@ -7,7 +7,12 @@ import { Context, InitialContext } from '../../..'; import link from '../../components/link'; import { redact } from '../../../lib/utils'; -const buildFields = ({ id, number, webUrl }) => ({ id, number, webUrl }); +const buildFields = ({ id, number, storybookUrl = undefined, webUrl = undefined }) => ({ + id, + number, + ...(storybookUrl && { storybookUrl }), + ...(webUrl && { webUrl }), +}); export default function fatalError( ctx: Context | InitialContext, @@ -26,9 +31,8 @@ export default function fatalError( runtimeMetadata, exitCode, exitCodeKey, - isolatorUrl, - cachedUrl, - build, + announcedBuild, + build = announcedBuild, buildCommand, } = ctx; @@ -54,8 +58,6 @@ export default function fatalError( exitCodeKey, errorType: errors.map((err) => err.name).join('\n'), errorMessage: stripAnsi(errors[0].message.split('\n')[0].trim()), - ...(isolatorUrl ? { isolatorUrl } : {}), - ...(cachedUrl ? { cachedUrl } : {}), ...(build && { build: buildFields(build) }), }, 'projectToken', diff --git a/node-src/ui/messages/errors/maxFileCountExceeded.stories.ts b/node-src/ui/messages/errors/maxFileCountExceeded.stories.ts new file mode 100644 index 000000000..41b67cade --- /dev/null +++ b/node-src/ui/messages/errors/maxFileCountExceeded.stories.ts @@ -0,0 +1,11 @@ +import { maxFileCountExceeded } from './maxFileCountExceeded'; + +export default { + title: 'CLI/Messages/Errors', +}; + +export const MaxFileCountExceeded = () => + maxFileCountExceeded({ + fileCount: 54_321, + maxFileCount: 20_000, + }); diff --git a/node-src/ui/messages/errors/maxFileCountExceeded.ts b/node-src/ui/messages/errors/maxFileCountExceeded.ts new file mode 100644 index 000000000..6487599c8 --- /dev/null +++ b/node-src/ui/messages/errors/maxFileCountExceeded.ts @@ -0,0 +1,19 @@ +import chalk from 'chalk'; +import { dedent } from 'ts-dedent'; + +import { error } from '../../components/icons'; + +export const maxFileCountExceeded = ({ + fileCount, + maxFileCount, +}: { + fileCount: number; + maxFileCount: number; +}) => + dedent(chalk` + ${error} {bold Attempted to upload too many files} + You're not allowed to upload more than ${maxFileCount} files per build. + Your Storybook contains ${fileCount} files. This is a very high number. + Do you have files in a static/public directory that shouldn't be there? + Contact customer support if you need to increase this limit. + `); diff --git a/node-src/ui/messages/errors/maxFileSizeExceeded.stories.ts b/node-src/ui/messages/errors/maxFileSizeExceeded.stories.ts new file mode 100644 index 000000000..a7cb75430 --- /dev/null +++ b/node-src/ui/messages/errors/maxFileSizeExceeded.stories.ts @@ -0,0 +1,8 @@ +import { maxFileSizeExceeded } from './maxFileSizeExceeded'; + +export default { + title: 'CLI/Messages/Errors', +}; + +export const MaxFileSizeExceeded = () => + maxFileSizeExceeded({ filePaths: ['index.js', 'main.js'], maxFileSize: 12345 }); diff --git a/node-src/ui/messages/errors/maxFileSizeExceeded.ts b/node-src/ui/messages/errors/maxFileSizeExceeded.ts new file mode 100644 index 000000000..ec2c274a9 --- /dev/null +++ b/node-src/ui/messages/errors/maxFileSizeExceeded.ts @@ -0,0 +1,19 @@ +import chalk from 'chalk'; +import { filesize } from 'filesize'; +import { dedent } from 'ts-dedent'; + +import { error } from '../../components/icons'; + +export const maxFileSizeExceeded = ({ + filePaths, + maxFileSize, +}: { + filePaths: string[]; + maxFileSize: number; +}) => + dedent(chalk` + ${error} {bold Attempted to exceed maximum file size} + You're attempting to upload files that exceed the maximum file size of ${filesize(maxFileSize)}. + Contact customer support if you need to increase this limit. + - ${filePaths.map((path) => path).join('\n- ')} + `); diff --git a/node-src/ui/messages/info/storybookPublished.stories.ts b/node-src/ui/messages/info/storybookPublished.stories.ts index 55e017638..9dbee8d3a 100644 --- a/node-src/ui/messages/info/storybookPublished.stories.ts +++ b/node-src/ui/messages/info/storybookPublished.stories.ts @@ -5,6 +5,11 @@ export default { }; export const StorybookPublished = () => + storybookPublished({ + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', + } as any); + +export const StorybookPrepared = () => storybookPublished({ build: { actualCaptureCount: undefined, @@ -14,6 +19,6 @@ export const StorybookPublished = () => errorCount: undefined, componentCount: 5, specCount: 8, - cachedUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/iframe.html', }, + storybookUrl: 'https://5d67dc0374b2e300209c41e7-pfkaemtlit.chromatic.com/', } as any); diff --git a/node-src/ui/messages/info/storybookPublished.ts b/node-src/ui/messages/info/storybookPublished.ts index 8254904a5..084a663d1 100644 --- a/node-src/ui/messages/info/storybookPublished.ts +++ b/node-src/ui/messages/info/storybookPublished.ts @@ -1,17 +1,23 @@ import chalk from 'chalk'; import { dedent } from 'ts-dedent'; -import { baseStorybookUrl } from '../../../lib/utils'; import { Context } from '../../../types'; import { info, success } from '../../components/icons'; import link from '../../components/link'; import { stats } from '../../tasks/snapshot'; -export default ({ build }: Pick) => { - const { components, stories } = stats({ build }); +export default ({ build, storybookUrl }: Pick) => { + if (build) { + const { components, stories } = stats({ build }); + return dedent(chalk` + ${success} {bold Storybook published} + We found ${components} with ${stories}. + ${info} View your Storybook at ${link(storybookUrl)} + `); + } + return dedent(chalk` ${success} {bold Storybook published} - We found ${components} with ${stories}. - ${info} View your Storybook at ${link(baseStorybookUrl(build.cachedUrl))} + ${info} View your Storybook at ${link(storybookUrl)} `); }; diff --git a/node-src/ui/tasks/upload.stories.ts b/node-src/ui/tasks/upload.stories.ts index c6691d394..cf66e6368 100644 --- a/node-src/ui/tasks/upload.stories.ts +++ b/node-src/ui/tasks/upload.stories.ts @@ -20,9 +20,6 @@ export default { decorators: [(storyFn: any) => task(storyFn())], }; -const isolatorUrl = 'https://5eb48280e78a12aeeaea33cf-kdypokzbrs.chromatic.com/iframe.html'; -// const storybookUrl = 'https://self-hosted-storybook.netlify.app'; - export const Initial = () => initial; export const DryRun = () => dryRun(); @@ -60,6 +57,14 @@ export const Starting = () => starting(); export const Uploading = () => uploading({ percentage: 42 }); -export const Success = () => success({ now: 0, startedAt: -54321, isolatorUrl } as any); +export const Success = () => + success({ + now: 0, + startedAt: -54321, + uploadedBytes: 1234567, + uploadedFiles: 42, + } as any); + +export const SuccessNoFiles = () => success({} as any); export const Failed = () => failed({ path: 'main.9e3e453142da82719bf4.bundle.js' }); diff --git a/node-src/ui/tasks/upload.ts b/node-src/ui/tasks/upload.ts index 193162bfb..c9f169e27 100644 --- a/node-src/ui/tasks/upload.ts +++ b/node-src/ui/tasks/upload.ts @@ -1,7 +1,8 @@ +import { filesize } from 'filesize'; import pluralize from 'pluralize'; import { getDuration } from '../../lib/tasks'; -import { baseStorybookUrl, progressBar, isPackageManifestFile } from '../../lib/utils'; +import { isPackageManifestFile, progressBar } from '../../lib/utils'; import { Context } from '../../types'; export const initial = { @@ -98,11 +99,15 @@ export const uploading = ({ percentage }: { percentage: number }) => ({ output: `${progressBar(percentage)} ${percentage}%`, }); -export const success = (ctx: Context) => ({ - status: 'success', - title: `Publish complete in ${getDuration(ctx)}`, - output: `View your Storybook at ${baseStorybookUrl(ctx.isolatorUrl)}`, -}); +export const success = (ctx: Context) => { + const files = pluralize('file', ctx.uploadedFiles, true); + const bytes = filesize(ctx.uploadedBytes || 0); + return { + status: 'success', + title: ctx.uploadedBytes ? `Publish complete in ${getDuration(ctx)}` : `Publish complete`, + output: ctx.uploadedBytes ? `Uploaded ${files} (${bytes})` : 'No new files to upload', + }; +}; export const failed = ({ path }: { path: string }) => ({ status: 'error', diff --git a/package.json b/package.json index eb938ec74..dd3d5b9a7 100644 --- a/package.json +++ b/package.json @@ -150,6 +150,7 @@ "execa": "^7.2.0", "fake-tag": "^2.0.0", "filesize": "^10.1.0", + "form-data": "^4.0.0", "fs-extra": "^10.0.0", "https-proxy-agent": "^7.0.2", "husky": "^7.0.0", @@ -202,6 +203,9 @@ }, "auto": { "baseBranch": "main", + "canary": { + "force": true + }, "plugins": [ "npm", "released", diff --git a/yarn.lock b/yarn.lock index ea2c2ff3c..29ad83171 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7905,6 +7905,15 @@ form-data@^3.0.0: combined-stream "^1.0.8" mime-types "^2.1.12" +form-data@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/form-data/-/form-data-4.0.0.tgz#93919daeaf361ee529584b9b31664dc12c9fa452" + integrity sha512-ETEklSGi5t0QMZuiXoA/Q6vcnxcLQP5vdugSpuAyi6SVGi2clPPp+xgEhuMaHC+zGgn31Kd235W35f7Hykkaww== + dependencies: + asynckit "^0.4.0" + combined-stream "^1.0.8" + mime-types "^2.1.12" + format@^0.2.0: version "0.2.2" resolved "https://registry.yarnpkg.com/format/-/format-0.2.2.tgz#d6170107e9efdc4ed30c9dc39016df942b5cb58b" From d58804a3585e49ebd129c7bb7ccbc343fe2344ab Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 12 Jan 2024 09:38:53 +0100 Subject: [PATCH 10/14] Run uploadBuild mutation in batches --- node-src/lib/upload.ts | 97 ++++++++++++++++++----------- node-src/tasks/upload.ts | 9 +-- node-src/ui/tasks/upload.stories.ts | 3 - node-src/ui/tasks/upload.ts | 6 -- 4 files changed, 63 insertions(+), 52 deletions(-) diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index f5ad83571..9d9a58e57 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -5,6 +5,9 @@ import { uploadFiles } from './uploadFiles'; import { maxFileCountExceeded } from '../ui/messages/errors/maxFileCountExceeded'; import { maxFileSizeExceeded } from '../ui/messages/errors/maxFileSizeExceeded'; +// This limit is imposed by the uploadBuild mutation +const MAX_FILES_PER_REQUEST = 1000; + const UploadBuildMutation = ` mutation UploadBuildMutation($buildId: ObjID!, $files: [FileUploadInput!]!, $zip: Boolean) { uploadBuild(buildId: $buildId, files: $files, zip: $zip) { @@ -80,62 +83,86 @@ export async function uploadBuild( onError?: (error: Error, path?: string) => void; } = {} ) { - const { uploadBuild } = await ctx.client.runQuery( - UploadBuildMutation, - { - buildId: ctx.announcedBuild.id, - files: files.map(({ contentLength, targetPath }) => ({ - contentLength, - filePath: targetPath, - })), - zip: ctx.options.zip, - } - ); + ctx.uploadedBytes = 0; + ctx.uploadedFiles = 0; - if (uploadBuild.userErrors.length) { - uploadBuild.userErrors.forEach((e) => { - if (e.__typename === 'MaxFileCountExceededError') { - ctx.log.error(maxFileCountExceeded(e)); - } else if (e.__typename === 'MaxFileSizeExceededError') { - ctx.log.error(maxFileSizeExceeded(e)); - } else { - ctx.log.error(e.message); + const targets: (TargetInfo & FileDesc)[] = []; + let zipTarget: (TargetInfo & { sentinelUrl: string }) | undefined; + + const batches = files.reduce((acc, file, fileIndex) => { + const batchIndex = Math.floor(fileIndex / MAX_FILES_PER_REQUEST); + if (!acc[batchIndex]) acc[batchIndex] = []; + acc[batchIndex].push(file); + return acc; + }, []); + + // The uploadBuild mutation has to run in batches to avoid hitting request/response payload limits + // or running out of memory. These run sequentially to avoid too many concurrent requests. + // The uploading itself still happens without batching, since it has its own concurrency limiter. + for (const batch of batches) { + const { uploadBuild } = await ctx.client.runQuery( + UploadBuildMutation, + { + buildId: ctx.announcedBuild.id, + files: batch.map(({ contentLength, targetPath }) => ({ + contentLength, + filePath: targetPath, + })), + zip: ctx.options.zip, } - }); - return options.onError?.(new Error('Upload rejected due to user error')); - } + ); - const targets = uploadBuild.info.targets.map((target) => { - const file = files.find((f) => f.targetPath === target.filePath); - return { ...file, ...target }; - }); + if (uploadBuild.userErrors.length) { + uploadBuild.userErrors.forEach((e) => { + if (e.__typename === 'MaxFileCountExceededError') { + ctx.log.error(maxFileCountExceeded(e)); + } else if (e.__typename === 'MaxFileSizeExceededError') { + ctx.log.error(maxFileSizeExceeded(e)); + } else { + ctx.log.error(e.message); + } + }); + return options.onError?.(new Error('Upload rejected due to user error')); + } + + targets.push( + ...uploadBuild.info.targets.map((target) => { + const file = batch.find((f) => f.targetPath === target.filePath); + return { ...file, ...target }; + }) + ); + zipTarget = uploadBuild.info.zipTarget; + } if (!targets.length) { ctx.log.debug('No new files to upload, continuing'); - return options.onComplete?.(0, 0); + return; } - options.onStart?.(); + // Uploading zero-length files is valid, so this might add up to 0. + const totalBytes = targets.reduce((sum, { contentLength }) => sum + contentLength, 0); - const total = targets.reduce((acc, { contentLength }) => acc + contentLength, 0); - if (uploadBuild.info.zipTarget) { + if (zipTarget) { try { const { path, size } = await makeZipFile(ctx, targets); - const compressionRate = (total - size) / total; + const compressionRate = totalBytes && (totalBytes - size) / totalBytes; ctx.log.debug(`Compression reduced upload size by ${Math.round(compressionRate * 100)}%`); - const target = { ...uploadBuild.info.zipTarget, contentLength: size, localPath: path }; + const target = { ...zipTarget, contentLength: size, localPath: path }; await uploadZip(ctx, target, (progress) => options.onProgress?.(progress, size)); await waitForUnpack(ctx, target.sentinelUrl); - return options.onComplete?.(size, targets.length); + ctx.uploadedBytes += size; + ctx.uploadedFiles += targets.length; + return; } catch (err) { ctx.log.debug({ err }, 'Error uploading zip, falling back to uploading individual files'); } } try { - await uploadFiles(ctx, targets, (progress) => options.onProgress?.(progress, total)); - return options.onComplete?.(total, targets.length); + await uploadFiles(ctx, targets, (progress) => options.onProgress?.(progress, totalBytes)); + ctx.uploadedBytes += totalBytes; + ctx.uploadedFiles += targets.length; } catch (e) { return options.onError?.(e, files.some((f) => f.localPath === e.message) && e.message); } diff --git a/node-src/tasks/upload.ts b/node-src/tasks/upload.ts index 93903c3be..8d40726ff 100644 --- a/node-src/tasks/upload.ts +++ b/node-src/tasks/upload.ts @@ -13,7 +13,6 @@ import { dryRun, validating, invalid, - preparing, tracing, bailed, traced, @@ -202,7 +201,7 @@ export const calculateFileHashes = async (ctx: Context, task: Task) => { export const uploadStorybook = async (ctx: Context, task: Task) => { if (ctx.skip) return; - transitionTo(preparing)(ctx, task); + transitionTo(starting)(ctx, task); const files = ctx.fileInfo.paths .map((path) => ({ @@ -213,21 +212,15 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { .filter((f) => f.contentLength); await uploadBuild(ctx, files, { - onStart: () => (task.output = starting().output), onProgress: throttle( (progress, total) => { const percentage = Math.round((progress / total) * 100); task.output = uploading({ percentage }).output; - ctx.options.experimental_onTaskProgress?.({ ...ctx }, { progress, total, unit: 'bytes' }); }, // Avoid spamming the logs with progress updates in non-interactive mode ctx.options.interactive ? 100 : ctx.env.CHROMATIC_OUTPUT_INTERVAL ), - onComplete: (uploadedBytes: number, uploadedFiles: number) => { - ctx.uploadedBytes = uploadedBytes; - ctx.uploadedFiles = uploadedFiles; - }, onError: (error: Error, path?: string) => { throw path === error.message ? new Error(failed({ path }).output) : error; }, diff --git a/node-src/ui/tasks/upload.stories.ts b/node-src/ui/tasks/upload.stories.ts index cf66e6368..5cae29880 100644 --- a/node-src/ui/tasks/upload.stories.ts +++ b/node-src/ui/tasks/upload.stories.ts @@ -6,7 +6,6 @@ import { hashing, initial, invalid, - preparing, starting, success, traced, @@ -51,8 +50,6 @@ export const Traced = () => traced({ onlyStoryFiles: Array.from({ length: 5 }) } export const Hashing = () => hashing(); -export const Preparing = () => preparing(); - export const Starting = () => starting(); export const Uploading = () => uploading({ percentage: 42 }); diff --git a/node-src/ui/tasks/upload.ts b/node-src/ui/tasks/upload.ts index c9f169e27..1c28f741f 100644 --- a/node-src/ui/tasks/upload.ts +++ b/node-src/ui/tasks/upload.ts @@ -81,12 +81,6 @@ export const hashing = () => ({ output: `Calculating file hashes`, }); -export const preparing = () => ({ - status: 'pending', - title: 'Publishing your built Storybook', - output: `Retrieving target location`, -}); - export const starting = () => ({ status: 'pending', title: 'Publishing your built Storybook', From 1e452d4015092aa126942a224e2e8f038e042d9f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 12 Jan 2024 11:55:02 +0100 Subject: [PATCH 11/14] Don't filter zero-length files and add tests --- node-src/tasks/upload.test.ts | 83 +++++++++++++++++++++++++++++++++++ node-src/tasks/upload.ts | 12 +++-- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index 3bfb5817a..c7e7148a8 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -329,6 +329,89 @@ describe('uploadStorybook', () => { expect(ctx.uploadedFiles).toBe(2); }); + it('batches calls to uploadBuild mutation', async () => { + const client = { runQuery: vi.fn() }; + client.runQuery.mockReturnValueOnce({ + uploadBuild: { + info: { + targets: Array.from({ length: 1000 }, (_, i) => ({ + contentType: 'application/javascript', + filePath: `${i}.js`, + formAction: `https://s3.amazonaws.com/presigned?${i}.js`, + formFields: {}, + })), + }, + userErrors: [], + }, + }); + client.runQuery.mockReturnValueOnce({ + uploadBuild: { + info: { + targets: [ + { + contentType: 'application/javascript', + filePath: `1000.js`, + formAction: `https://s3.amazonaws.com/presigned?1000.js`, + formFields: {}, + }, + ], + }, + userErrors: [], + }, + }); + + createReadStreamMock.mockReturnValue({ pipe: vi.fn() } as any); + http.fetch.mockReturnValue({ ok: true }); + progress.mockReturnValue({ on: vi.fn() } as any); + + const fileInfo = { + lengths: Array.from({ length: 1001 }, (_, i) => ({ knownAs: `${i}.js`, contentLength: i })), + paths: Array.from({ length: 1001 }, (_, i) => `${i}.js`), + total: Array.from({ length: 1001 }, (_, i) => i).reduce((a, v) => a + v), + }; + const ctx = { + client, + env, + log, + http, + sourceDir: '/static/', + options: {}, + fileInfo, + announcedBuild: { id: '1' }, + } as any; + await uploadStorybook(ctx, {} as any); + + expect(client.runQuery).toHaveBeenCalledTimes(2); + expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { + buildId: '1', + files: Array.from({ length: 1000 }, (_, i) => ({ + contentLength: i, + filePath: `${i}.js`, + })), + }); + expect(client.runQuery).toHaveBeenCalledWith(expect.stringMatching(/UploadBuildMutation/), { + buildId: '1', + files: [{ contentLength: 1000, filePath: `1000.js` }], // 0-based index makes this file #1001 + }); + expect(http.fetch).toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?0.js', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), + expect.objectContaining({ retries: 0 }) + ); + expect(http.fetch).toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?999.js', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), + expect.objectContaining({ retries: 0 }) + ); + expect(http.fetch).toHaveBeenCalledWith( + 'https://s3.amazonaws.com/presigned?1000.js', + expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), + expect.objectContaining({ retries: 0 }) + ); + expect(ctx.uploadedBytes).toBe(500500); + expect(ctx.uploadedFiles).toBe(1001); + }); + it.skip('calls experimental_onTaskProgress with progress', async () => { const client = { runQuery: vi.fn() }; client.runQuery.mockReturnValue({ diff --git a/node-src/tasks/upload.ts b/node-src/tasks/upload.ts index 8d40726ff..e6e65435b 100644 --- a/node-src/tasks/upload.ts +++ b/node-src/tasks/upload.ts @@ -203,13 +203,11 @@ export const uploadStorybook = async (ctx: Context, task: Task) => { if (ctx.skip) return; transitionTo(starting)(ctx, task); - const files = ctx.fileInfo.paths - .map((path) => ({ - contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === path).contentLength, - localPath: join(ctx.sourceDir, path), - targetPath: path, - })) - .filter((f) => f.contentLength); + const files = ctx.fileInfo.paths.map((path) => ({ + contentLength: ctx.fileInfo.lengths.find(({ knownAs }) => knownAs === path).contentLength, + localPath: join(ctx.sourceDir, path), + targetPath: path, + })); await uploadBuild(ctx, files, { onProgress: throttle( From a373f1f4eed4def483ad99a8a87fe80929c2319f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 12 Jan 2024 14:29:40 +0100 Subject: [PATCH 12/14] Log batches --- node-src/lib/upload.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index 9d9a58e57..76bc7063c 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -99,7 +99,9 @@ export async function uploadBuild( // The uploadBuild mutation has to run in batches to avoid hitting request/response payload limits // or running out of memory. These run sequentially to avoid too many concurrent requests. // The uploading itself still happens without batching, since it has its own concurrency limiter. - for (const batch of batches) { + for (const [index, batch] of batches.entries()) { + ctx.log.debug(`Running uploadBuild batch ${index + 1} / ${batches.length}`); + const { uploadBuild } = await ctx.client.runQuery( UploadBuildMutation, { From a36bcdbd851f0354d8889409e3a549cc940c741e Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 15 Jan 2024 17:08:56 +0100 Subject: [PATCH 13/14] Skip empty files when uploading individual files --- node-src/lib/uploadFiles.ts | 9 ++++++++- .../warnings/skippingEmptyFiles.stories.ts | 14 ++++++++++++++ .../ui/messages/warnings/skippingEmptyFiles.ts | 17 +++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts create mode 100644 node-src/ui/messages/warnings/skippingEmptyFiles.ts diff --git a/node-src/lib/uploadFiles.ts b/node-src/lib/uploadFiles.ts index b0e41e465..6d1f3b602 100644 --- a/node-src/lib/uploadFiles.ts +++ b/node-src/lib/uploadFiles.ts @@ -5,6 +5,7 @@ import { createReadStream } from 'fs'; import pLimit from 'p-limit'; import progress from 'progress-stream'; import { Context, FileDesc, TargetInfo } from '../types'; +import skippingEmptyFiles from '../ui/messages/warnings/skippingEmptyFiles'; export async function uploadFiles( ctx: Context, @@ -15,8 +16,14 @@ export async function uploadFiles( const limitConcurrency = pLimit(10); let totalProgress = 0; + const nonEmptyTargets = targets.filter(({ contentLength }) => contentLength > 0); + if (nonEmptyTargets.length !== targets.length) { + const emptyFiles = targets.filter(({ contentLength }) => contentLength === 0); + ctx.log.warn(skippingEmptyFiles({ emptyFiles })); + } + await Promise.all( - targets.map(({ contentLength, filePath, formAction, formFields, localPath }) => { + nonEmptyTargets.map(({ contentLength, filePath, formAction, formFields, localPath }) => { let fileProgress = 0; // The bytes uploaded for this this particular file ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)})`); diff --git a/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts b/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts new file mode 100644 index 000000000..07a66c3bb --- /dev/null +++ b/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts @@ -0,0 +1,14 @@ +import skippingEmptyFiles from './skippingEmptyFiles'; + +export default { + title: 'CLI/Messages/Warnings', +}; + +export const SkippingEmptyFiles = () => + skippingEmptyFiles({ + emptyFiles: Array.from({ length: 3 }, (_, i) => ({ + contentLength: 0, + localPath: `file${i}.js`, + targetPath: `file${i}.js`, + })), + }); diff --git a/node-src/ui/messages/warnings/skippingEmptyFiles.ts b/node-src/ui/messages/warnings/skippingEmptyFiles.ts new file mode 100644 index 000000000..4601ee271 --- /dev/null +++ b/node-src/ui/messages/warnings/skippingEmptyFiles.ts @@ -0,0 +1,17 @@ +import chalk from 'chalk'; +import pluralize from 'pluralize'; +import { dedent } from 'ts-dedent'; + +import { warning } from '../../components/icons'; +import { FileDesc } from '../../../types'; + +export default ({ emptyFiles }: { emptyFiles: FileDesc[] }) => { + const listing = chalk`\n{dim →} ${emptyFiles.map((f) => f.targetPath).join(chalk`\n{dim →} `)}`; + return dedent(chalk` + ${warning} {bold Not uploading empty files} + Found ${pluralize('empty files', emptyFiles.length, true)} in your built Storybook:${listing} + Uploading empty files is not supported except when using a zip file. + You can ignore this warning if your Storybook doesn't need these files. + Otherwise, configure Chromatic with the {bold zip} option. + `); +}; From 130a553ec92e8f2a9ab8002fe12eabfbafaf2aa0 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 15 Jan 2024 20:51:37 +0100 Subject: [PATCH 14/14] Fix test and number of uploaded files --- node-src/lib/upload.ts | 10 ++++++++-- node-src/lib/uploadFiles.ts | 9 +-------- node-src/tasks/upload.test.ts | 6 ++++-- .../ui/messages/warnings/skippingEmptyFiles.stories.ts | 2 +- node-src/ui/messages/warnings/skippingEmptyFiles.ts | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/node-src/lib/upload.ts b/node-src/lib/upload.ts index 76bc7063c..02e03d149 100644 --- a/node-src/lib/upload.ts +++ b/node-src/lib/upload.ts @@ -4,6 +4,7 @@ import { uploadZip, waitForUnpack } from './uploadZip'; import { uploadFiles } from './uploadFiles'; import { maxFileCountExceeded } from '../ui/messages/errors/maxFileCountExceeded'; import { maxFileSizeExceeded } from '../ui/messages/errors/maxFileSizeExceeded'; +import { skippingEmptyFiles } from '../ui/messages/warnings/skippingEmptyFiles'; // This limit is imposed by the uploadBuild mutation const MAX_FILES_PER_REQUEST = 1000; @@ -162,9 +163,14 @@ export async function uploadBuild( } try { - await uploadFiles(ctx, targets, (progress) => options.onProgress?.(progress, totalBytes)); + const nonEmptyFiles = targets.filter(({ contentLength }) => contentLength > 0); + if (nonEmptyFiles.length !== targets.length) { + const emptyFiles = targets.filter(({ contentLength }) => contentLength === 0); + ctx.log.warn(skippingEmptyFiles({ emptyFiles })); + } + await uploadFiles(ctx, nonEmptyFiles, (progress) => options.onProgress?.(progress, totalBytes)); ctx.uploadedBytes += totalBytes; - ctx.uploadedFiles += targets.length; + ctx.uploadedFiles += nonEmptyFiles.length; } catch (e) { return options.onError?.(e, files.some((f) => f.localPath === e.message) && e.message); } diff --git a/node-src/lib/uploadFiles.ts b/node-src/lib/uploadFiles.ts index 6d1f3b602..b0e41e465 100644 --- a/node-src/lib/uploadFiles.ts +++ b/node-src/lib/uploadFiles.ts @@ -5,7 +5,6 @@ import { createReadStream } from 'fs'; import pLimit from 'p-limit'; import progress from 'progress-stream'; import { Context, FileDesc, TargetInfo } from '../types'; -import skippingEmptyFiles from '../ui/messages/warnings/skippingEmptyFiles'; export async function uploadFiles( ctx: Context, @@ -16,14 +15,8 @@ export async function uploadFiles( const limitConcurrency = pLimit(10); let totalProgress = 0; - const nonEmptyTargets = targets.filter(({ contentLength }) => contentLength > 0); - if (nonEmptyTargets.length !== targets.length) { - const emptyFiles = targets.filter(({ contentLength }) => contentLength === 0); - ctx.log.warn(skippingEmptyFiles({ emptyFiles })); - } - await Promise.all( - nonEmptyTargets.map(({ contentLength, filePath, formAction, formFields, localPath }) => { + targets.map(({ contentLength, filePath, formAction, formFields, localPath }) => { let fileProgress = 0; // The bytes uploaded for this this particular file ctx.log.debug(`Uploading ${filePath} (${filesize(contentLength)})`); diff --git a/node-src/tasks/upload.test.ts b/node-src/tasks/upload.test.ts index c7e7148a8..6398f46fa 100644 --- a/node-src/tasks/upload.test.ts +++ b/node-src/tasks/upload.test.ts @@ -393,7 +393,9 @@ describe('uploadStorybook', () => { buildId: '1', files: [{ contentLength: 1000, filePath: `1000.js` }], // 0-based index makes this file #1001 }); - expect(http.fetch).toHaveBeenCalledWith( + + // Empty files are not uploaded + expect(http.fetch).not.toHaveBeenCalledWith( 'https://s3.amazonaws.com/presigned?0.js', expect.objectContaining({ body: expect.any(FormData), method: 'POST' }), expect.objectContaining({ retries: 0 }) @@ -409,7 +411,7 @@ describe('uploadStorybook', () => { expect.objectContaining({ retries: 0 }) ); expect(ctx.uploadedBytes).toBe(500500); - expect(ctx.uploadedFiles).toBe(1001); + expect(ctx.uploadedFiles).toBe(1000); }); it.skip('calls experimental_onTaskProgress with progress', async () => { diff --git a/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts b/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts index 07a66c3bb..09ec9e8f7 100644 --- a/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts +++ b/node-src/ui/messages/warnings/skippingEmptyFiles.stories.ts @@ -1,4 +1,4 @@ -import skippingEmptyFiles from './skippingEmptyFiles'; +import { skippingEmptyFiles } from './skippingEmptyFiles'; export default { title: 'CLI/Messages/Warnings', diff --git a/node-src/ui/messages/warnings/skippingEmptyFiles.ts b/node-src/ui/messages/warnings/skippingEmptyFiles.ts index 4601ee271..56cfca883 100644 --- a/node-src/ui/messages/warnings/skippingEmptyFiles.ts +++ b/node-src/ui/messages/warnings/skippingEmptyFiles.ts @@ -5,7 +5,7 @@ import { dedent } from 'ts-dedent'; import { warning } from '../../components/icons'; import { FileDesc } from '../../../types'; -export default ({ emptyFiles }: { emptyFiles: FileDesc[] }) => { +export const skippingEmptyFiles = ({ emptyFiles }: { emptyFiles: FileDesc[] }) => { const listing = chalk`\n{dim →} ${emptyFiles.map((f) => f.targetPath).join(chalk`\n{dim →} `)}`; return dedent(chalk` ${warning} {bold Not uploading empty files}