From 0c3d7021200af1de40662bf50b9ab09431ba5380 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Fri, 3 Jul 2026 11:38:09 +0530 Subject: [PATCH] fix: align _iframe_shim with the canonical @percy/sdk-utils iframe helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit percy/cli#2319 made @percy/sdk-utils the single source of truth for the iframe helpers (resolveMaxFrameDepth, resolveIgnoreSelectors, normalizeIgnoreSelectors, isUnsupportedIframeSrc). Our shim already prefers those exports when the linked sdk-utils provides them, but the local fallbacks (and their tests) encoded a different contract, so the SDK regression run — which links sdk-utils from percy/cli master — failed: - resolveMaxFrameDepth: canonical reads `options.maxIframeDepth` (the public SnapshotOptions key) and treats invalid/<1 values as the default; the local copy read a made-up `maxFrameDepth` key and clamped negatives to 0. - normalizeIgnoreSelectors: canonical takes a raw value; the local copy aliased resolveIgnoreSelectors and took an options object. - The local fallbacks became dead code when the canonical exports were present, collapsing coverage below the 100% thresholds. Rewrite the fallbacks to be contract-identical to canonical (including the `percy.config.snapshot` fallback and the ws:/wss:/ftp: unsupported schemes), reusing the already-published `clampIframeDepth` for the shared clamping semantics. Expose the fallbacks as `_localFallbacks` and run the same contract test suite against both the public exports and the fallbacks, so behavior and 100% coverage hold whether the linked @percy/sdk-utils is the published 1.31.14 or percy/cli master. Co-Authored-By: Claude Fable 5 --- _iframe_shim.js | 79 ++++++++----- test/iframe-helpers.test.js | 225 ++++++++++++++++++++++-------------- 2 files changed, 188 insertions(+), 116 deletions(-) diff --git a/_iframe_shim.js b/_iframe_shim.js index c7fe997..59a3e77 100644 --- a/_iframe_shim.js +++ b/_iframe_shim.js @@ -1,41 +1,45 @@ -// Local shim: extends @percy/sdk-utils with helpers the published 1.31.14-beta.3 -// does not yet export. SDK code expects these names; we provide them locally -// until sdk-utils is updated. +// Local shim: extends @percy/sdk-utils with helpers the published 1.31.14 +// does not yet export. SDK code expects these names; we delegate to the +// canonical sdk-utils exports (the single source of truth) when the linked +// version provides them, and fall back to contract-identical local copies +// otherwise. const utils = require('@percy/sdk-utils'); +// MIRROR: must match UNSUPPORTED_IFRAME_SRCS in @percy/sdk-utils. Only used +// when the linked sdk-utils predates the canonical export. const BROWSER_INTERNAL_PREFIXES = [ 'about:', 'chrome:', 'chrome-extension:', 'devtools:', 'edge:', 'opera:', 'view-source:', 'data:', 'javascript:', 'blob:', // legacy IE-era schemes that still appear on adversarial pages - 'vbscript:', 'file:' + 'vbscript:', 'file:', 'ws:', 'wss:', 'ftp:' ]; +// Canonical contract: per-snapshot `maxIframeDepth` wins over the global +// `percy.config.snapshot.maxIframeDepth`, then the value is clamped to +// [1, HARD_MAX_IFRAME_DEPTH] (invalid/<1 -> default). The published 1.31.14 +// already exports `clampIframeDepth`, so clamping semantics are shared. function resolveMaxFrameDepth(options = {}) { - const requested = options.maxFrameDepth ?? options.maxIframeDepth; - // sdk-utils >= 1.31.14-beta.4 provides DEFAULT_MAX_IFRAME_DEPTH and - // HARD_MAX_IFRAME_DEPTH. The `?? N` arms only fire on older sdk-utils - // versions, hence defensive. - /* istanbul ignore next: defensive fallback for older @percy/sdk-utils */ - const def = utils.DEFAULT_MAX_IFRAME_DEPTH ?? 10; - /* istanbul ignore next: defensive fallback for older @percy/sdk-utils */ - const hard = utils.HARD_MAX_IFRAME_DEPTH ?? 25; - const value = requested == null ? def : Number(requested); - if (Number.isNaN(value)) return def; - return Math.max(0, Math.min(value, hard)); + let raw = options.maxIframeDepth; + if (raw == null) raw = utils.percy.config && utils.percy.config.snapshot && utils.percy.config.snapshot.maxIframeDepth; + return utils.clampIframeDepth(raw); } -function resolveIgnoreSelectors(options = {}) { - const sel = options.ignoreIframeSelectors ?? options.ignoreSelectors; - if (!sel) return []; - if (Array.isArray(sel)) return sel.filter(s => typeof s === 'string' && s.length); - // `!sel` above already filtered empty strings, so a string here is - // guaranteed non-empty. - if (typeof sel === 'string') return [sel]; +// Canonical contract: takes a raw value (array | string | unset), NOT an +// options object, and normalizes it into a clean string[]. +function normalizeIgnoreSelectors(value) { + if (!value) return []; + if (Array.isArray(value)) return value.filter(s => typeof s === 'string' && s.length); + if (typeof value === 'string') return [value]; return []; } -function normalizeIgnoreSelectors(options = {}) { - return resolveIgnoreSelectors(options); +// Canonical contract: per-snapshot `ignoreIframeSelectors` (legacy alias +// `ignoreSelectors`) wins; when absent, fall back to the global +// `percy.config.snapshot.ignoreIframeSelectors`. Always returns a string[]. +function resolveIgnoreSelectors(options = {}) { + const perSnapshot = normalizeIgnoreSelectors(options.ignoreIframeSelectors ?? options.ignoreSelectors); + if (perSnapshot.length) return perSnapshot; + return normalizeIgnoreSelectors(utils.percy.config && utils.percy.config.snapshot && utils.percy.config.snapshot.ignoreIframeSelectors); } function isUnsupportedIframeSrc(src) { @@ -44,9 +48,28 @@ function isUnsupportedIframeSrc(src) { return BROWSER_INTERNAL_PREFIXES.some(p => s.startsWith(p)); } +// Prefer the canonical sdk-utils implementation when the linked version +// exports it; otherwise use the local fallback above. Which arm runs depends +// solely on the linked @percy/sdk-utils version, so coverage of the arms is +// mode-dependent — the fallbacks themselves are covered directly through +// `_localFallbacks` below. +/* istanbul ignore next: arm taken depends on the linked @percy/sdk-utils version */ +function preferCanonical(name, fallback) { + return utils[name] || fallback; +} + module.exports = Object.assign({}, utils, { - resolveMaxFrameDepth: utils.resolveMaxFrameDepth || resolveMaxFrameDepth, - resolveIgnoreSelectors: utils.resolveIgnoreSelectors || resolveIgnoreSelectors, - normalizeIgnoreSelectors: utils.normalizeIgnoreSelectors || normalizeIgnoreSelectors, - isUnsupportedIframeSrc: utils.isUnsupportedIframeSrc || isUnsupportedIframeSrc + resolveMaxFrameDepth: preferCanonical('resolveMaxFrameDepth', resolveMaxFrameDepth), + resolveIgnoreSelectors: preferCanonical('resolveIgnoreSelectors', resolveIgnoreSelectors), + normalizeIgnoreSelectors: preferCanonical('normalizeIgnoreSelectors', normalizeIgnoreSelectors), + isUnsupportedIframeSrc: preferCanonical('isUnsupportedIframeSrc', isUnsupportedIframeSrc) }); + +// Exposed for tests only: the local fallbacks must stay contract-identical to +// the canonical sdk-utils exports no matter which arm preferCanonical picked. +module.exports._localFallbacks = { + resolveMaxFrameDepth, + resolveIgnoreSelectors, + normalizeIgnoreSelectors, + isUnsupportedIframeSrc +}; diff --git a/test/iframe-helpers.test.js b/test/iframe-helpers.test.js index 20c53b8..69d4ceb 100644 --- a/test/iframe-helpers.test.js +++ b/test/iframe-helpers.test.js @@ -483,100 +483,149 @@ describe('findIframeByPercyId', () => { }); describe('_iframe_shim', () => { - describe('resolveIgnoreSelectors', () => { - it('returns [] when no selectors are set', () => { - expect(shim.resolveIgnoreSelectors({})).toEqual([]); - expect(shim.resolveIgnoreSelectors()).toEqual([]); - }); - - it('returns the array unchanged when an array of strings is passed', () => { - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: ['.a', '.b'] })).toEqual(['.a', '.b']); - }); - - it('filters out non-string and empty entries from an array', () => { - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: ['.a', '', null, 7, '.b'] })).toEqual(['.a', '.b']); - }); - - it('wraps a single string selector in an array', () => { - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: '.solo' })).toEqual(['.solo']); - }); - - it('returns [] for an empty string selector', () => { - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: '' })).toEqual([]); - }); - - it('falls back to the legacy ignoreSelectors key', () => { - expect(shim.resolveIgnoreSelectors({ ignoreSelectors: ['.legacy'] })).toEqual(['.legacy']); - }); - - it('returns [] for unsupported types (number, object, boolean)', () => { - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: 42 })).toEqual([]); - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: { a: 1 } })).toEqual([]); - expect(shim.resolveIgnoreSelectors({ ignoreIframeSelectors: true })).toEqual([]); - }); - }); - - describe('normalizeIgnoreSelectors', () => { - it('is an alias of resolveIgnoreSelectors', () => { - expect(shim.normalizeIgnoreSelectors({ ignoreIframeSelectors: ['.x'] })).toEqual(['.x']); - expect(shim.normalizeIgnoreSelectors()).toEqual([]); - }); - }); - - describe('resolveMaxFrameDepth', () => { - // sdk-utils exposes DEFAULT_MAX_IFRAME_DEPTH / HARD_MAX_IFRAME_DEPTH; - // pull them at runtime so this test tracks the dependency rather than - // hardcoding numbers that will drift when sdk-utils bumps. - const utils = require('@percy/sdk-utils'); - const DEFAULT = utils.DEFAULT_MAX_IFRAME_DEPTH ?? 10; - const HARD = utils.HARD_MAX_IFRAME_DEPTH ?? 25; - - it('returns the default when not supplied', () => { - expect(shim.resolveMaxFrameDepth({})).toBe(DEFAULT); - expect(shim.resolveMaxFrameDepth()).toBe(DEFAULT); - }); - - it('returns the explicit value when within range', () => { - const inRange = Math.min(DEFAULT, HARD); - expect(shim.resolveMaxFrameDepth({ maxFrameDepth: inRange })).toBe(inRange); - }); - - it('accepts the legacy maxIframeDepth key', () => { - const inRange = Math.min(DEFAULT, HARD); - expect(shim.resolveMaxFrameDepth({ maxIframeDepth: inRange })).toBe(inRange); - }); - - it('clamps negative values up to 0', () => { - expect(shim.resolveMaxFrameDepth({ maxFrameDepth: -5 })).toBe(0); - }); - - it('clamps overflow values down to the hard cap', () => { - expect(shim.resolveMaxFrameDepth({ maxFrameDepth: HARD + 1000 })).toBe(HARD); - }); - - it('falls back to default when value is not numeric', () => { - expect(shim.resolveMaxFrameDepth({ maxFrameDepth: 'banana' })).toBe(DEFAULT); + // sdk-utils is the single source of truth for these values; pull them at + // runtime so this test tracks the dependency rather than hardcoding numbers + // that will drift when sdk-utils bumps. + const utils = require('@percy/sdk-utils'); + const DEFAULT = utils.DEFAULT_MAX_IFRAME_DEPTH; + const HARD = utils.HARD_MAX_IFRAME_DEPTH; + + // The canonical helpers fall back to `percy.config`, which other suites in + // this process can populate via the healthcheck flow — start each spec from + // a clean slate and restore whatever was there afterwards. + let ogConfig; + beforeEach(() => { + ogConfig = utils.percy.config; + delete utils.percy.config; + }); + + afterEach(() => { + utils.percy.config = ogConfig; + }); + + // The shim delegates to the canonical @percy/sdk-utils implementations when + // the linked version exports them and uses local fallbacks otherwise. Run + // the same contract suite against both so the fallbacks can never drift + // from canonical behavior (and both stay fully covered in either mode). + Object.entries({ + 'public exports': shim, + 'local fallbacks': shim._localFallbacks + }).forEach(([label, impl]) => { + describe(`resolveIgnoreSelectors (${label})`, () => { + it('returns [] when no selectors are set', () => { + expect(impl.resolveIgnoreSelectors({})).toEqual([]); + expect(impl.resolveIgnoreSelectors()).toEqual([]); + }); + + it('returns the array unchanged when an array of strings is passed', () => { + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: ['.a', '.b'] })).toEqual(['.a', '.b']); + }); + + it('filters out non-string and empty entries from an array', () => { + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: ['.a', '', null, 7, '.b'] })).toEqual(['.a', '.b']); + }); + + it('wraps a single string selector in an array', () => { + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: '.solo' })).toEqual(['.solo']); + }); + + it('returns [] for an empty string selector', () => { + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: '' })).toEqual([]); + }); + + it('falls back to the legacy ignoreSelectors key', () => { + expect(impl.resolveIgnoreSelectors({ ignoreSelectors: ['.legacy'] })).toEqual(['.legacy']); + }); + + it('returns [] for unsupported types (number, object, boolean)', () => { + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: 42 })).toEqual([]); + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: { a: 1 } })).toEqual([]); + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: true })).toEqual([]); + }); + + it('falls back to the global percy config when no option is set', () => { + utils.percy.config = { snapshot: { ignoreIframeSelectors: ['.from-config'] } }; + expect(impl.resolveIgnoreSelectors({})).toEqual(['.from-config']); + }); + + it('prefers the per-snapshot option over the global percy config', () => { + utils.percy.config = { snapshot: { ignoreIframeSelectors: ['.from-config'] } }; + expect(impl.resolveIgnoreSelectors({ ignoreIframeSelectors: ['.opt'] })).toEqual(['.opt']); + }); }); - }); - describe('isUnsupportedIframeSrc', () => { - it('returns true for null/empty', () => { - expect(shim.isUnsupportedIframeSrc(null)).toBe(true); - expect(shim.isUnsupportedIframeSrc('')).toBe(true); + describe(`normalizeIgnoreSelectors (${label})`, () => { + it('normalizes a raw value (not an options object) into a string[]', () => { + expect(impl.normalizeIgnoreSelectors(['.x'])).toEqual(['.x']); + expect(impl.normalizeIgnoreSelectors('.solo')).toEqual(['.solo']); + expect(impl.normalizeIgnoreSelectors(['.a', '', null, 7, '.b'])).toEqual(['.a', '.b']); + }); + + it('returns [] for unset, empty, and unsupported values', () => { + expect(impl.normalizeIgnoreSelectors()).toEqual([]); + expect(impl.normalizeIgnoreSelectors('')).toEqual([]); + expect(impl.normalizeIgnoreSelectors({ a: 1 })).toEqual([]); + }); }); - it('returns true for browser-internal and legacy schemes', () => { - expect(shim.isUnsupportedIframeSrc('about:blank')).toBe(true); - expect(shim.isUnsupportedIframeSrc('javascript:void(0)')).toBe(true); - expect(shim.isUnsupportedIframeSrc('data:text/html,foo')).toBe(true); - expect(shim.isUnsupportedIframeSrc('blob:http://example.com/x')).toBe(true); - expect(shim.isUnsupportedIframeSrc('vbscript:msgbox')).toBe(true); - expect(shim.isUnsupportedIframeSrc('file:///etc/passwd')).toBe(true); + describe(`resolveMaxFrameDepth (${label})`, () => { + it('returns the default when not supplied', () => { + expect(impl.resolveMaxFrameDepth({})).toBe(DEFAULT); + expect(impl.resolveMaxFrameDepth()).toBe(DEFAULT); + }); + + it('returns the explicit maxIframeDepth value when within range', () => { + const inRange = Math.min(DEFAULT, HARD); + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: inRange })).toBe(inRange); + }); + + it('returns the default for invalid values (negative, zero, non-numeric)', () => { + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: -5 })).toBe(DEFAULT); + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: 0 })).toBe(DEFAULT); + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: 'banana' })).toBe(DEFAULT); + }); + + it('clamps overflow values down to the hard cap', () => { + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: HARD + 1000 })).toBe(HARD); + }); + + it('floors fractional values', () => { + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: 2.9 })).toBe(2); + }); + + it('falls back to the global percy config when no option is set', () => { + utils.percy.config = { snapshot: { maxIframeDepth: Math.min(DEFAULT, HARD) + 1 } }; + expect(impl.resolveMaxFrameDepth({})).toBe(Math.min(DEFAULT, HARD) + 1); + }); + + it('prefers the per-snapshot option over the global percy config', () => { + utils.percy.config = { snapshot: { maxIframeDepth: HARD } }; + const inRange = Math.min(DEFAULT, HARD); + expect(impl.resolveMaxFrameDepth({ maxIframeDepth: inRange })).toBe(inRange); + }); }); - it('returns false for http(s)', () => { - expect(shim.isUnsupportedIframeSrc('http://example.com')).toBe(false); - expect(shim.isUnsupportedIframeSrc('https://example.com/page')).toBe(false); + describe(`isUnsupportedIframeSrc (${label})`, () => { + it('returns true for null/empty', () => { + expect(impl.isUnsupportedIframeSrc(null)).toBe(true); + expect(impl.isUnsupportedIframeSrc('')).toBe(true); + }); + + it('returns true for browser-internal, legacy, and non-http schemes', () => { + expect(impl.isUnsupportedIframeSrc('about:blank')).toBe(true); + expect(impl.isUnsupportedIframeSrc('javascript:void(0)')).toBe(true); + expect(impl.isUnsupportedIframeSrc('data:text/html,foo')).toBe(true); + expect(impl.isUnsupportedIframeSrc('blob:http://example.com/x')).toBe(true); + expect(impl.isUnsupportedIframeSrc('vbscript:msgbox')).toBe(true); + expect(impl.isUnsupportedIframeSrc('file:///etc/passwd')).toBe(true); + expect(impl.isUnsupportedIframeSrc('ws://example.com/socket')).toBe(true); + expect(impl.isUnsupportedIframeSrc('ftp://example.com/file')).toBe(true); + }); + + it('returns false for http(s)', () => { + expect(impl.isUnsupportedIframeSrc('http://example.com')).toBe(false); + expect(impl.isUnsupportedIframeSrc('https://example.com/page')).toBe(false); + }); }); }); });