Skip to content

fix: align _iframe_shim with the canonical @percy/sdk-utils iframe helpers#734

Merged
pranavz28 merged 1 commit into
masterfrom
PER-9772_adapt-iframe-shim-to-sdk-utils
Jul 3, 2026
Merged

fix: align _iframe_shim with the canonical @percy/sdk-utils iframe helpers#734
pranavz28 merged 1 commit into
masterfrom
PER-9772_adapt-iframe-shim-to-sdk-utils

Conversation

@pranavz28

Copy link
Copy Markdown
Contributor

Root cause

The PER-9772 SDK regression dispatches this repo's test.yml with all @percy/* packages linked from a percy/cli master build. percy/cli#2319 made @percy/sdk-utils the single source of truth for the iframe helpers, so in that mode the shim's utils.X || localX delegation started picking the canonical implementations — whose contract differs from what the local fallbacks (and their tests) encoded:

  • resolveMaxFrameDepth — canonical reads options.maxIframeDepth (the public SnapshotOptions key) and treats invalid/< 1 values as the default (3); the local copy read a locally-invented maxFrameDepth key and clamped negatives to 0. Hence Expected 3 to be 10 (the maxFrameDepth key was ignored → default) and Expected 3 to be 0 (canonical returns the default for negatives).
  • normalizeIgnoreSelectors — canonical takes a raw value (array | string | unset); the local copy aliased resolveIgnoreSelectors and took an options object. Hence Expected $.length = 0 to equal 1 for normalizeIgnoreSelectors({ ignoreIframeSelectors: ['.x'] }).
  • Coverage — with the canonical exports present, the local fallback bodies became dead code, collapsing _iframe_shim.js to ~13% and failing the 100% thresholds.

Normal pull_request CI kept passing because the lockfile's published @percy/sdk-utils@1.31.14 doesn't export these helpers yet, so the local fallbacks were always used.

Failing run: https://github.com/percy/percy-protractor/actions/runs/28640552076

Fix

  • Rewrote the local fallbacks in _iframe_shim.js to be contract-identical to the canonical sdk-utils helpers: maxIframeDepth key, percy.config.snapshot.* fallback, invalid/< 1 → default, floor + hard cap via the already-published clampIframeDepth, value-based normalizeIgnoreSelectors, and the ws:/wss:/ftp: additions to the unsupported-scheme list.
  • Kept the delegation preferring the canonical exports when present (single source of truth), and exposed the fallbacks as _localFallbacks for tests.
  • Updated test/iframe-helpers.test.js to assert the canonical contract, and to run the same contract suite against both the public exports and the local fallbacks — so the fallbacks can never silently drift from canonical behavior, and 100% coverage of the shim holds no matter which arm is active.

Verification

Ran the _iframe_shim/iframe-helpers suite (jasmine + nyc) in both dependency modes:

Mode Before After
Published @percy/sdk-utils@1.31.14 (lockfile) 43 specs, 0 failures, shim 100% 68 specs, 0 failures, shim 100% (stmts/branch/funcs/lines)
@percy/sdk-utils linked from percy/cli master (yarn build + yarn link) 43 specs, 3 failures, shim 13% coverage — exact CI failures 68 specs, 0 failures, shim 100% (stmts/branch/funcs/lines)

yarn lint passes. index.js is untouched, so its coverage profile in the full protractor run is unchanged.

🤖 Generated with Claude Code

…lpers

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 <noreply@anthropic.com>
@pranavz28
pranavz28 requested a review from a team as a code owner July 3, 2026 06:09
@pranavz28
pranavz28 merged commit 746dbfe into master Jul 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants