From ec4c31066bae957918aa7eedc83c1451873bebc7 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Mon, 10 Aug 2026 23:08:40 +0530 Subject: [PATCH 1/3] fix(core): stop compressing discovered resources in place Under PERCY_GZIP, processSnapshotResources compressed each resource by overwriting `resource.content` and `resource.sha`. Those objects are the same ones held by the build-wide resource cache, so the first snapshot's upload pass replaced every cached body with its gzip bytes. Every later snapshot then had those entries replayed to the browser via Fetch.fulfillRequest using the resource's original headers -- e.g. `content-type: text/css` with no `content-encoding`. The browser sniffs a body that does not match the declared type and blocks it (ERR_BLOCKED_BY_ORB), so the stylesheet is never parsed. Anything it references is therefore never requested and never captured: a stylesheet carrying @font-face yields no webfont for any snapshot after the first, and the renderer 404s the font it still needs. Compress a copy and leave the cached entry holding the original bytes. Cache hit rates are unchanged; only the mutation is gone. --- packages/core/src/discovery.js | 27 ++++++++++++++++-------- packages/core/test/discovery.test.js | 31 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index ba15c00b6..9061f3248 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -230,12 +230,21 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { const kept = []; for (let index = 0; index < resources.length; index++) { const resource = resources[index]; + // Compression must never be applied in place. These resource objects are + // the same ones held by the build-wide resource cache, which replays them + // to the browser via Fetch.fulfillRequest using their original headers + // (e.g. `content-type: text/css`, no `content-encoding`). Overwriting + // `content` with gzip bytes leaves the cache serving compressed bodies + // labelled as plain text, so the browser cannot parse them -- a stylesheet + // poisoned this way yields no @font-face and no background-image, and the + // resources they reference are never requested and so never captured for + // any later snapshot. Compress a copy and leave the cached entry intact. + let uploaded = resource; try { - const alreadyZipped = isGzipped(resource.content); - /* istanbul ignore next: very hard to mock true */ - if (!alreadyZipped) { - resource.content = Pako.gzip(resource.content); - resource.sha = sha256hash(resource.content); + /* istanbul ignore else: alreadyZipped is very hard to mock true */ + if (!isGzipped(resource.content)) { + const content = Pako.gzip(resource.content); + uploaded = { ...resource, content, sha: sha256hash(content) }; log.debug(`- Gzipped resource: ${resource.url}`); } } catch (error) { @@ -247,9 +256,9 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { continue; } - // resource.content is guaranteed by the try block above (either just - // assigned from Pako.gzip, or alreadyZipped was true). - const size = resource.content.length; + // uploaded.content is guaranteed by the try block above (either just + // assigned from Pako.gzip, or the resource was already compressed). + const size = uploaded.content.length; // Root (DOM HTML) and log resources are required for a valid snapshot; // shipping an oversized one and letting the API surface a clear error // is better than silently dropping it here. @@ -263,7 +272,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { ); continue; } - kept.push(resource); + kept.push(uploaded); } resources = kept; } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index d9008c842..234c1fa98 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -839,6 +839,37 @@ describe('Discovery', () => { ); }); + it('keeps cached resources uncompressed when PERCY_GZIP is enabled', async () => { + // Compressing a resource in place mutates the entry held by the build-wide + // resource cache. Later snapshots then have the compressed body replayed to + // the browser under the resource's original `text/css` content type, so the + // browser cannot parse the stylesheet, never sees the @font-face it declares, + // and never requests the font -- leaving it out of every later snapshot. + process.env.PERCY_GZIP = true; + + server.reply('/style.css', () => [200, 'text/css', [ + '@font-face { font-family: "test"; src: url("/font.woff") format("woff"); }', + 'body { font-family: "test", "sans-serif"; }' + ].join('')]); + + await percy.snapshot({ name: 'one', url: 'http://localhost:8000', domSnapshot: testDOM }); + await percy.snapshot({ name: 'two', url: 'http://localhost:8000', domSnapshot: testDOM }); + + await percy.idle(); + + let font = jasmine.objectContaining({ + id: sha256hash(Pako.gzip('')), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/font.woff' + }) + }); + + expect(captured[0]).toEqual(jasmine.arrayContaining([font])); + // the second snapshot's stylesheet is served from the cache -- it must still + // arrive parseable so the font it references is discovered again + expect(captured[1]).toEqual(jasmine.arrayContaining([font])); + }); + it('captures resource larger than 25MB raw when PERCY_GZIP is enabled', async () => { process.env.PERCY_GZIP = true; const largeCSS = 'A'.repeat(30_000_000); From d3e141006ef7bdbdda252d53d36badd2c96d9c7a Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Mon, 10 Aug 2026 23:12:10 +0530 Subject: [PATCH 2/3] chore(core): condense comments --- packages/core/src/discovery.js | 14 +++----------- packages/core/test/discovery.test.js | 10 +++------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 9061f3248..6b9581c1e 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -230,15 +230,8 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { const kept = []; for (let index = 0; index < resources.length; index++) { const resource = resources[index]; - // Compression must never be applied in place. These resource objects are - // the same ones held by the build-wide resource cache, which replays them - // to the browser via Fetch.fulfillRequest using their original headers - // (e.g. `content-type: text/css`, no `content-encoding`). Overwriting - // `content` with gzip bytes leaves the cache serving compressed bodies - // labelled as plain text, so the browser cannot parse them -- a stylesheet - // poisoned this way yields no @font-face and no background-image, and the - // resources they reference are never requested and so never captured for - // any later snapshot. Compress a copy and leave the cached entry intact. + // Never compress in place: these objects are shared with the resource + // cache, which must keep replaying the original bytes to the browser. let uploaded = resource; try { /* istanbul ignore else: alreadyZipped is very hard to mock true */ @@ -256,8 +249,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { continue; } - // uploaded.content is guaranteed by the try block above (either just - // assigned from Pako.gzip, or the resource was already compressed). + // uploaded.content is guaranteed by the try block above. const size = uploaded.content.length; // Root (DOM HTML) and log resources are required for a valid snapshot; // shipping an oversized one and letting the API surface a clear error diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 234c1fa98..fd6ba579d 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -840,11 +840,8 @@ describe('Discovery', () => { }); it('keeps cached resources uncompressed when PERCY_GZIP is enabled', async () => { - // Compressing a resource in place mutates the entry held by the build-wide - // resource cache. Later snapshots then have the compressed body replayed to - // the browser under the resource's original `text/css` content type, so the - // browser cannot parse the stylesheet, never sees the @font-face it declares, - // and never requests the font -- leaving it out of every later snapshot. + // A cached stylesheet compressed in place is replayed as gzip bytes under its + // original text/css type, so the browser never parses it or its @font-face. process.env.PERCY_GZIP = true; server.reply('/style.css', () => [200, 'text/css', [ @@ -865,8 +862,7 @@ describe('Discovery', () => { }); expect(captured[0]).toEqual(jasmine.arrayContaining([font])); - // the second snapshot's stylesheet is served from the cache -- it must still - // arrive parseable so the font it references is discovered again + // snapshot two's stylesheet comes from the cache and must still be parseable expect(captured[1]).toEqual(jasmine.arrayContaining([font])); }); From d8035dfeb8c2fbaf9d3458134b38d062172e74da Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Tue, 11 Aug 2026 04:28:34 +0530 Subject: [PATCH 3/3] fix(core): clarify gzip comments after removing in-place mutation The istanbul pragma still named a variable the previous commit deleted, and the size comment restated what the code does rather than why. Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/src/discovery.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 6b9581c1e..5a63c8011 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -234,7 +234,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { // cache, which must keep replaying the original bytes to the browser. let uploaded = resource; try { - /* istanbul ignore else: alreadyZipped is very hard to mock true */ + /* istanbul ignore else: an already-gzipped resource is very hard to mock */ if (!isGzipped(resource.content)) { const content = Pako.gzip(resource.content); uploaded = { ...resource, content, sha: sha256hash(content) }; @@ -249,7 +249,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { continue; } - // uploaded.content is guaranteed by the try block above. + // Either the gzipped copy or, when already gzipped, the resource itself. const size = uploaded.content.length; // Root (DOM HTML) and log resources are required for a valid snapshot; // shipping an oversized one and letting the API surface a clear error