feat(sdk-utils): own readiness — shallow-merge precedence + move waitForReady from @percy/dom (PER-7348)#2236
Merged
Conversation
…(PER-7348)
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Shivanshu-07
changed the title
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) <noreply@anthropic.com>
…PER-7348)" This reverts commit d43c9ba.
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.
rishigupta1599
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related changes that consolidate readiness ownership in
@percy/sdk-utils:1.
getReadinessConfigshallow-merge fix (4ece9cc6)Used
options.readiness || percy.config?.snapshot?.readiness || {}— surfaced during SDK PR review (percy-cypress #1087, percy-ember #1149) as having two latent failure modes:options = { readiness: {} }short-circuits the fallback and discards the entire.percy.ymlconfig.snapshot.readiness.preset: disabledandpercySnapshot('x', { readiness: { stabilityWindowMs: 500 } }), the disabled state is silently lost.Switched to shallow-merge — per-snapshot keys win, unspecified keys inherited.
2. Move readiness implementation to
@percy/sdk-utils(d43c9bae)The 556-line readiness implementation lived in
@percy/dom. The SDK-facing helpers (getReadinessConfig,waitForReadyScript,isReadinessDisabled) already lived in@percy/sdk-utils. Splitting "implementation" from "orchestration" across two packages was confusing.Moved:
packages/dom/src/readiness.js→packages/sdk-utils/src/readiness-browser.jspackages/dom/test/readiness-helpers.test.js→packages/sdk-utils/test/packages/dom/src/index.jsnow re-exportswaitForReadyfrom@percy/sdk-utils/readiness-browser.@percy/sdk-utils/package.jsonadds the file tofilesand the exports map.Why the source lives in sdk-utils but is consumed via @percy/dom: the code uses browser globals (MutationObserver, document, performance, etc.) so it can't run in Node.
@percy/dom's rollup build inlines it into the browser bundle that ships viafetchPercyDOM(). End users of@percy/domstill get a self-contained bundle — no runtime dep on sdk-utils — because the bundle is already inlined.Verified:
yarn buildinpackages/domproduces adist/bundle.jsthat still containsfunction waitForReadyand the full readiness implementation.PercyDOM.waitForReadyglobal remains identical for every SDK caller — no SDK changes required.Test plan
packages/sdk-utils/test/index.test.jscover the shallow-merge precedence: empty per-snapshot, partial override inheritance, globalpreset: disabledinheritance, and the existing per-snapshot-wins case.readiness-helpers.test.jsmoved alongside the source it tests; imports updated to../src/readiness-browser.js.yarn buildinpackages/domsucceeds; readiness implementation inlined intodist/bundle.js.import { waitForReady } from '@percy/dom'still works.Net effect
Single source of truth per concern in
@percy/sdk-utils:@percy/sdk-utils/readiness-browser(namedwaitForReady)waitForReadyScript(cfg, { callback? })getReadinessConfig(options)isReadinessDisabled(options)Future readiness changes (new checks, preset additions, log shape) land once and flow everywhere via the existing dependency chain.
🤖 Generated with Claude Code