From 994d24acc3c40a15eb1914c04e504152b21a2fab Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Wed, 6 May 2026 00:36:30 +0530 Subject: [PATCH 1/3] feat: add mergeSnapshotOptions utility to @percy/sdk-utils Centralizes the .percy.yml config merge logic that was duplicated across all 10 Percy JS SDKs. The function merges percy.config.snapshot options with per-snapshot options, giving snapshot options priority. Co-Authored-By: Claude Opus 4.6 --- packages/sdk-utils/src/index.js | 2 + .../sdk-utils/src/merge-snapshot-options.js | 10 +++++ packages/sdk-utils/test/index.test.js | 45 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 packages/sdk-utils/src/merge-snapshot-options.js diff --git a/packages/sdk-utils/src/index.js b/packages/sdk-utils/src/index.js index 1659886d1..7fba38706 100644 --- a/packages/sdk-utils/src/index.js +++ b/packages/sdk-utils/src/index.js @@ -10,6 +10,7 @@ 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 mergeSnapshotOptions from './merge-snapshot-options.js'; import { waitForReadyScript, getReadinessConfig, @@ -47,6 +48,7 @@ export { captureAutomateScreenshot, postBuildEvents, getResponsiveWidths, + mergeSnapshotOptions, DEFAULT_MAX_IFRAME_DEPTH, HARD_MAX_IFRAME_DEPTH, clampIframeDepth, diff --git a/packages/sdk-utils/src/merge-snapshot-options.js b/packages/sdk-utils/src/merge-snapshot-options.js new file mode 100644 index 000000000..14339eabe --- /dev/null +++ b/packages/sdk-utils/src/merge-snapshot-options.js @@ -0,0 +1,10 @@ +import percy from './percy-info.js'; + +// Merges .percy.yml config snapshot options with per-snapshot options. +// Per-snapshot options take priority over config options. +export function mergeSnapshotOptions(options) { + const configOptions = percy?.config?.snapshot || {}; + return { ...configOptions, ...options }; +} + +export default mergeSnapshotOptions; diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index 3bf19be3a..98499272e 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -936,4 +936,49 @@ describe('SDK Utils', () => { expect(result).toBe(null); }); }); + + describe('mergeSnapshotOptions(options)', () => { + let { mergeSnapshotOptions } = utils; + + beforeEach(async () => { + await helpers.setupTest(); + await utils.isPercyEnabled(); + }); + + it('merges config snapshot options with per-snapshot options', () => { + const result = mergeSnapshotOptions({ enableJavaScript: true }); + expect(result.enableJavaScript).toBe(true); + expect(result.widths).toEqual([375, 1280]); + }); + + it('gives per-snapshot options priority over config', () => { + const result = mergeSnapshotOptions({ widths: [768] }); + expect(result.widths).toEqual([768]); + }); + + it('returns config options when no per-snapshot options are provided', () => { + const result = mergeSnapshotOptions(); + expect(result.widths).toEqual([375, 1280]); + }); + + it('returns empty object when config.snapshot is undefined and no options given', () => { + const savedConfig = utils.percy.config; + utils.percy.config = { ...savedConfig, snapshot: undefined }; + + const result = mergeSnapshotOptions(); + expect(result).toEqual({}); + + utils.percy.config = savedConfig; + }); + + it('returns only per-snapshot options when config.snapshot is undefined', () => { + const savedConfig = utils.percy.config; + utils.percy.config = { ...savedConfig, snapshot: undefined }; + + const result = mergeSnapshotOptions({ enableJavaScript: true }); + expect(result).toEqual({ enableJavaScript: true }); + + utils.percy.config = savedConfig; + }); + }); }); From 33cf5cbbd1e26d86fd727068c5b8d5d72c661016 Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Tue, 16 Jun 2026 20:07:26 +0530 Subject: [PATCH 2/3] refactor(sdk-utils): default mergeSnapshotOptions param; document shallow-merge intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR review feedback on mergeSnapshotOptions: - Use a default parameter (options = {}) instead of relying on spreading undefined. - Document that the shallow merge is deliberate — for parity with the non-JS SDKs and getReadinessConfig, and because the CLI does the authoritative deep config merge server-side. Ref: PER-8053 --- packages/sdk-utils/src/merge-snapshot-options.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/sdk-utils/src/merge-snapshot-options.js b/packages/sdk-utils/src/merge-snapshot-options.js index 14339eabe..1ed820eb0 100644 --- a/packages/sdk-utils/src/merge-snapshot-options.js +++ b/packages/sdk-utils/src/merge-snapshot-options.js @@ -2,7 +2,15 @@ import percy from './percy-info.js'; // Merges .percy.yml config snapshot options with per-snapshot options. // Per-snapshot options take priority over config options. -export function mergeSnapshotOptions(options) { +// +// This is a deliberate shallow merge, for two reasons: +// 1. Parity — the non-JS SDKs (python/ruby/java/.net) all shallow-merge +// config with per-call options, and this package's own getReadinessConfig +// (serialize-dom.js) does the same. JS must behave identically. +// 2. The CLI performs the authoritative deep config merge server-side, so the +// SDK only needs top-level precedence: a per-snapshot key fully overrides +// its config counterpart before the DOM is serialized. +export function mergeSnapshotOptions(options = {}) { const configOptions = percy?.config?.snapshot || {}; return { ...configOptions, ...options }; } From 465b72e798844f70fb7984399f80d81adeb62b0d Mon Sep 17 00:00:00 2001 From: rishigupta1599 Date: Wed, 17 Jun 2026 00:06:40 +0530 Subject: [PATCH 3/3] fix(sdk-utils): deep-merge config and per-snapshot options Addresses the review finding that the shallow merge dropped nested config keys: a per-snapshot override of one nested key (e.g. discovery.disableCache) no longer discards the config's sibling nested keys. Nested plain objects are merged recursively; per-snapshot values win at the leaves and arrays are replaced (not concatenated). Adds tests for nested merge + array replacement. Ref: PER-8053 --- .../sdk-utils/src/merge-snapshot-options.js | 32 ++++++++++++++----- packages/sdk-utils/test/index.test.js | 24 ++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/packages/sdk-utils/src/merge-snapshot-options.js b/packages/sdk-utils/src/merge-snapshot-options.js index 1ed820eb0..d4868d798 100644 --- a/packages/sdk-utils/src/merge-snapshot-options.js +++ b/packages/sdk-utils/src/merge-snapshot-options.js @@ -1,18 +1,34 @@ import percy from './percy-info.js'; +function isPlainObject(value) { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + +// Recursively merge `override` onto `base`. Plain (non-array) objects are merged +// key-by-key so overriding one nested key keeps the base's sibling keys; arrays, +// scalars, null and functions from `override` replace the base value wholesale. +function deepMerge(base, override) { + const result = { ...base }; + for (const key of Object.keys(override)) { + const baseVal = base[key]; + const overrideVal = override[key]; + result[key] = isPlainObject(baseVal) && isPlainObject(overrideVal) + ? deepMerge(baseVal, overrideVal) + : overrideVal; + } + return result; +} + // Merges .percy.yml config snapshot options with per-snapshot options. // Per-snapshot options take priority over config options. // -// This is a deliberate shallow merge, for two reasons: -// 1. Parity — the non-JS SDKs (python/ruby/java/.net) all shallow-merge -// config with per-call options, and this package's own getReadinessConfig -// (serialize-dom.js) does the same. JS must behave identically. -// 2. The CLI performs the authoritative deep config merge server-side, so the -// SDK only needs top-level precedence: a per-snapshot key fully overrides -// its config counterpart before the DOM is serialized. +// The merge is deep: nested objects (e.g. `discovery`) are merged recursively so +// a per-snapshot override of one nested key does not drop the config's sibling +// nested keys. At the leaves, per-snapshot values win; arrays are replaced, not +// concatenated. export function mergeSnapshotOptions(options = {}) { const configOptions = percy?.config?.snapshot || {}; - return { ...configOptions, ...options }; + return deepMerge(configOptions, options); } export default mergeSnapshotOptions; diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index 98499272e..5362f2592 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -980,5 +980,29 @@ describe('SDK Utils', () => { utils.percy.config = savedConfig; }); + + it('deep-merges nested objects, keeping config sibling keys not overridden', () => { + const savedConfig = utils.percy.config; + utils.percy.config = { + ...savedConfig, + snapshot: { discovery: { networkIdleTimeout: 50, disableCache: false } } + }; + + const result = mergeSnapshotOptions({ discovery: { disableCache: true } }); + // per-snapshot wins on the overridden nested key, config sibling key survives + expect(result.discovery).toEqual({ networkIdleTimeout: 50, disableCache: true }); + + utils.percy.config = savedConfig; + }); + + it('replaces (does not concatenate) arrays from per-snapshot options', () => { + const savedConfig = utils.percy.config; + utils.percy.config = { ...savedConfig, snapshot: { widths: [375, 1280] } }; + + const result = mergeSnapshotOptions({ widths: [768] }); + expect(result.widths).toEqual([768]); + + utils.percy.config = savedConfig; + }); }); });