Merge pull request #168 from chromaui/steven/dont-rebufferize-archives

Cypress: Pass archive along instead of re-bufferizing it

.changeset/friendly-avocados-hammer.md

@@ -0,0 +1,7 @@
+---
+'@chromatic-com/playwright': patch
+'@chromatic-com/cypress': patch
+'@chromatic-com/shared-e2e': patch
+---
+
+Pass Cypress archive along instead of re-bufferizing; refactored resource-archiving code slightly

packages/cypress/src/index.ts

@@ -1,7 +1,7 @@
 import type { elementNode } from 'rrweb-snapshot';
 import CDP, { Version } from 'chrome-remote-interface';
 import {
-  Watcher,
+  ResourceArchiver,
   writeTestResult,
   ChromaticStorybookParameters,
   ResourceArchive,
@@ -35,19 +35,6 @@ const writeArchives = async ({
   pageUrl,
   viewport,
 }: WriteArchivesParams) => {
-  const bufferedArchiveList = Object.entries(resourceArchive).map(([key, value]) => {
-    return [
-      key,
-      {
-        ...value,
-        // we can't use Buffer in the browser (when we collect the responses)
-        // so we go through one by one here and bufferize them
-        // @ts-expect-error will fix when Cypress has its own package
-        body: Buffer.from(value.body, 'utf8'),
-      },
-    ];
-  });
-
   const allSnapshots = Object.fromEntries(
     // manual snapshots can be given a name; otherwise, just use the snapshot's place in line as the name
     domSnapshots.map(({ name, snapshot }, index) => [
@@ -66,16 +53,16 @@ const writeArchives = async ({
       viewport,
     },
     allSnapshots,
-    Object.fromEntries(bufferedArchiveList),
+    resourceArchive,
     chromaticStorybookParams
   );
 };
 
-// using a single Watcher instance across all tests (for the test run)
+// using a single ResourceArchiver instance across all tests (for the test run)
 // each time a test completes, we'll save to disk whatever archives are there at that point.
 // This should be safe since the same resource from the same URL should be the same during the entire test run.
 // Cypress doesn't give us a way to share variables between the "before test" and "after test" lifecycle events on the server.
-let watcher: Watcher = null;
+let resourceArchiver: ResourceArchiver = null;
 
 let host = '';
 let port = 0;
@@ -95,9 +82,9 @@ const setupNetworkListener = async ({
       target: webSocketDebuggerUrl,
     });
 
-    if (!watcher) {
-      watcher = new Watcher(cdp, allowedDomains);
-      await watcher.watch();
+    if (!resourceArchiver) {
+      resourceArchiver = new ResourceArchiver(cdp, allowedDomains);
+      await resourceArchiver.watch();
     }
   } catch (err) {
     console.log('err', err);
@@ -108,11 +95,11 @@ const setupNetworkListener = async ({
 
 const saveArchives = (archiveInfo: WriteParams) => {
   return new Promise((resolve) => {
-    // the watcher's archives come from the server, everything else (DOM snapshots, test info, etc) comes from the browser
-    // notice we're not calling + awaiting watcher.idle() here...
+    // the resourceArchiver's archives come from the server, everything else (DOM snapshots, test info, etc) comes from the browser
+    // notice we're not calling + awaiting resourceArchiver.idle() here...
     // that's because in Cypress, cy.visit() waits until all resources have loaded before finishing
     // so at this point (after the test) we're confident that the resources are all there already without having to wait more
-    return writeArchives({ ...archiveInfo, resourceArchive: watcher.archive }).then(() => {
+    return writeArchives({ ...archiveInfo, resourceArchive: resourceArchiver.archive }).then(() => {
       resolve(null);
     });
   });

packages/playwright/src/createResourceArchive.ts

@@ -1,6 +1,6 @@
 import type { Page } from 'playwright';
 import {
-  Watcher,
+  ResourceArchiver,
   ResourceArchive,
   DEFAULT_GLOBAL_RESOURCE_ARCHIVE_TIMEOUT_MS,
   logger,
@@ -43,12 +43,12 @@ export const createResourceArchive = async ({
 }): Promise<() => Promise<ResourceArchive>> => {
   const cdpClient = await page.context().newCDPSession(page);
 
-  const watcher = new Watcher(cdpClient, assetDomains);
-  await watcher.watch();
+  const resourceArchiver = new ResourceArchiver(cdpClient, assetDomains);
+  await resourceArchiver.watch();
 
   return async () => {
     await idle(page, networkTimeout ?? DEFAULT_GLOBAL_RESOURCE_ARCHIVE_TIMEOUT_MS);
 
-    return watcher.archive;
+    return resourceArchiver.archive;
   };
 };

packages/shared/src/index.ts

@@ -1,4 +1,4 @@
-export * from './resource-archive';
+export * from './resource-archiver';
 export * from './write-archive';
 export * from './utils/analytics';
 export * from './utils/logger';

packages/shared/src/resource-archive/index.ts → packages/shared/src/resource-archiver/index.ts

@@ -22,7 +22,7 @@ interface CDPClient {
   send: (eventName: keyof Protocol.CommandParameters, payload?: any) => Promise<any>;
 }
 
-export class Watcher {
+export class ResourceArchiver {
   public archive: ResourceArchive = {};
 
   private client: CDPClient;
@@ -110,40 +110,17 @@ export class Watcher {
 
     // Pausing a response stage with a response
     if (responseStatusCode) {
-      if ([301, 302, 307, 308].includes(responseStatusCode)) {
-        await this.clientSend(request, 'Fetch.continueRequest', {
+      await this.handleSuccessfulResponse(
+        {
+          request,
           requestId,
-          interceptResponse: true,
-        });
-        return;
-      }
-
-      const result = await this.clientSend(request, 'Fetch.getResponseBody', {
-        requestId,
-      });
-      // Something has gone wrong and will be logged above
-      if (result === null) {
-        return;
-      }
-      const { body, base64Encoded } = result;
-
-      // If the Content-Type header is present, let's capture it.
-      const contentTypeHeader: Protocol.Fetch.HeaderEntry = responseHeaders.find(
-        ({ name }) => name.toLowerCase() === 'content-type'
+          responseStatusCode,
+          responseStatusText,
+          responseHeaders,
+        },
+        requestUrl,
+        isRequestFromAllowedDomain
       );
-
-      // No need to capture the response of the top level page request
-      const isFirstRequest = requestUrl.toString() === this.firstUrl.toString();
-      if (isRequestFromAllowedDomain && !isFirstRequest) {
-        this.archive[request.url] = {
-          statusCode: responseStatusCode,
-          statusText: responseStatusText,
-          body: Buffer.from(body, base64Encoded ? 'base64' : 'utf8'),
-          contentType: contentTypeHeader?.value,
-        };
-      }
-
-      await this.clientSend(request, 'Fetch.continueRequest', { requestId });
       return;
     }
 
@@ -152,4 +129,51 @@ export class Watcher {
       interceptResponse: true,
     });
   }
+
+  private async handleSuccessfulResponse(
+    requestPausedPayload: Pick<
+      Protocol.Fetch.requestPausedPayload,
+      'request' | 'requestId' | 'responseStatusCode' | 'responseStatusText' | 'responseHeaders'
+    >,
+    requestUrl: URL,
+    isRequestFromAllowedDomain: boolean
+  ) {
+    const { request, requestId, responseStatusCode, responseStatusText, responseHeaders } =
+      requestPausedPayload;
+
+    if ([301, 302, 307, 308].includes(responseStatusCode)) {
+      await this.clientSend(request, 'Fetch.continueRequest', {
+        requestId,
+        interceptResponse: true,
+      });
+      return;
+    }
+
+    const result = await this.clientSend(request, 'Fetch.getResponseBody', {
+      requestId,
+    });
+    // Something has gone wrong and will be logged above
+    if (result === null) {
+      return;
+    }
+    const { body, base64Encoded } = result;
+
+    // If the Content-Type header is present, let's capture it.
+    const contentTypeHeader: Protocol.Fetch.HeaderEntry = responseHeaders.find(
+      ({ name }) => name.toLowerCase() === 'content-type'
+    );
+
+    // No need to capture the response of the top level page request
+    const isFirstRequest = requestUrl.toString() === this.firstUrl.toString();
+    if (isRequestFromAllowedDomain && !isFirstRequest) {
+      this.archive[request.url] = {
+        statusCode: responseStatusCode,
+        statusText: responseStatusText,
+        body: Buffer.from(body, base64Encoded ? 'base64' : 'utf8'),
+        contentType: contentTypeHeader?.value,
+      };
+    }
+
+    await this.clientSend(request, 'Fetch.continueRequest', { requestId });
+  }
 }

packages/shared/src/write-archive/archive-file.test.ts

@@ -1,5 +1,5 @@
 import { ArchiveFile } from './archive-file';
-import type { ArchiveResponse, UrlString } from '../resource-archive';
+import type { ArchiveResponse, UrlString } from '../resource-archiver';
 
 const imgPng =
   'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';

packages/shared/src/write-archive/archive-file.ts

@@ -2,7 +2,7 @@ import mime from 'mime';
 import path from 'path';
 import { createHash } from 'node:crypto';
 import { logger } from '../utils/logger';
-import type { ArchiveResponse, UrlString } from '../resource-archive';
+import type { ArchiveResponse, UrlString } from '../resource-archiver';
 
 const RESERVED_PATHNAMES: string[] = [
   // This is a list of filenames that are reserved for Storybook as a dev target.

packages/shared/src/write-archive/index.ts

@@ -3,7 +3,7 @@ import { outputFile, ensureDir, outputJSONFile } from '../utils/filePaths';
 import { logger } from '../utils/logger';
 import { ArchiveFile } from './archive-file';
 import { DOMSnapshot } from './dom-snapshot';
-import type { ResourceArchive } from '../resource-archive';
+import type { ResourceArchive } from '../resource-archiver';
 import type { ChromaticStorybookParameters } from '../types';
 import { Viewport } from '../utils/viewport';
 import { snapshotFileName, snapshotId } from './snapshot-files';