fix(dom): keep readiness gate async-free so TestCafe t.eval accepts the bundle

[prklm10]

Fixes PER-10121

Root cause

The @percy/dom bundle is injected into the browser verbatim by SDKs. @percy/testcafe/index.js:29 does:

await t.eval(new Function(await utils.fetchPercyDOM()), { boundTestRun: t });

TestCafe's t.eval/ClientFunction machinery statically rejects any async/await or generator syntax in the evaluated source.

The readiness gate shipped in 1.31.14 (PER-7348, #2184) added two async functions — runAllChecks and waitForReady in packages/dom/src/readiness.js — which are the only async/await in the entire dom source. Because the rollup babel target (browsers: ['last 2 versions and supports async-functions']) intentionally does not down-level async, those tokens flow straight into dist/bundle.js. Every @percy/testcafe snapshot then fails with:

eval code, arguments or dependencies cannot contain generators or "async/await" syntax (use Promises instead).
  at percySnapshot (node_modules/@percy/testcafe/index.js:29:13)

and the build finalizes with Failure: Snapshot command was not called. Playwright/other SDKs are unaffected because they don't route the bundle through TestCafe's restrictive evaluator.

Bundle token profile (published):

Version	size	async	await	generators
1.31.13 (last good)	47 KB	0	0	0
1.31.14	101 KB	2	2	0
1.32.4 (latest)	105 KB	2	2	0

Fix

Rewrite runAllChecks and waitForReady to explicit Promise chains (return Promise.all(...), Promise.race(...).catch(...).then(...)). Behavior is unchanged — both were already Promise-returning at every call site (runAllChecks(...).then(...), await waitForReady(...)), and the try/catch → .catch/.then mapping preserves the timeout/abort/post-processing semantics exactly. The rebuilt bundle is back to 0 async / 0 await / 0 generators, matching the 1.31.13 profile that TestCafe accepts.

Add a package-local packages/dom/src/.eslintrc that forbids async/await/generators via no-restricted-syntax, so this browser-injection contract can't silently regress again (which is exactly how it shipped).

Testing

• Full @percy/dom suite green: 902 tests pass, including the readiness suite (waitForReady returns a Promise, stays sync even when readiness config is present, timeout/graceful-degradation cases).
• Rebuilt dist/bundle.js verified async/await/generator-free.
• ESLint guard verified: fails an async sample with the PER-10121 message, passes the rewritten readiness.js.

🤖 Generated with Claude Code

[prklm10]

Claude Code Review (automated) — 4 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[prklm10]

[Low] Sync throws now escape waitForReady instead of landing in result.error

Previously waitForReady was async, so a synchronous throw during check setup became a rejection captured by the catch. Now runAllChecks runs synchronously before .catch attaches, so a sync throw (e.g. in checkFontReady, whose body executes outside a Promise executor) propagates out as an exception. All in-repo callers are protected, but third-party SDK code calling PercyDOM.waitForReady(cfg).then(...) directly would see an escaped exception instead of a captured error.

Suggestion: wrap the call async-free to convert sync throws into rejections:

new Promise(resolve => resolve(runAllChecks(config, result, aborted))).then(() => { settled = true; }),

Reviewer: stack:code-reviewer

[prklm10]

[Low] The guard protects source, not the shipped artifact

The real contract is "the built bundle is async-free"; a future bundled dependency, babel helper, or rollup-target change could reintroduce async into dist/bundle.js without any src/ lint violation.

Suggestion: add a small regression test (or CI step) that parses the built bundle with acorn (already in the dependency tree) and asserts zero async/generator nodes — it would have caught the original 1.31.14 regression.

Reviewer: stack:code-reviewer

[prklm10]

[Low] Top-level for await slips through the selectors

for await (const x of y) at module top level is a ForOfStatement[await=true] — none of the four selectors match it there (inside a function, :function[async=true] catches the container). Verified against ESLint 7.32.

Suggestion: add a fifth selector: ForOfStatement[await=true].

Reviewer: stack:code-reviewer

[prklm10]

[Low] Guard silently dies under an ESLint flat-config migration

Extensionless .eslintrc relies on legacy cascading config, which ESLint 9 flat config removes — if the repo migrates, this guard stops applying silently with no error.

Suggestion: add a one-line note in this header comment so a flat-config migration doesn't silently drop the guard (a bundle-AST regression test would also backstop this).

Reviewer: stack:code-reviewer

[prklm10]

Claude Code PR Review

PR: #2346 • Head: a102dac • Reviewers: stack:code-reviewer

Summary

Rewrites the only two async functions in packages/dom/src (runAllChecks, waitForReady in readiness.js) as explicit Promise chains so the shipped @percy/dom bundle stays free of async/await/generator syntax (which TestCafe's t.eval statically rejects — PER-10121), and adds a package-local .eslintrc no-restricted-syntax guard so the contract can't silently regress.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No secrets in diff
High	Security	Authentication/authorization checks present	N/A	No auth surface touched
High	Security	Input validation and sanitization	N/A	No new external input handling
High	Security	No IDOR — resource ownership validated	N/A	No resource access changes
High	Security	No SQL injection (parameterized queries)	N/A	No database code
High	Correctness	Logic is correct, handles edge cases	Pass	.catch(...).then(...) chain verified behaviorally equivalent to the old try/await/catch (error capture, timeout marking, _expectedChecks cleanup, passed computation). Rebuilt bundle AST-verified: 0 async / 0 await / 0 generators
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	One Low note: a synchronous throw during check setup now escapes waitForReady instead of landing in result.error (Finding 1); all in-repo callers verified protected
High	Correctness	No race conditions or concurrency issues	Pass	settled/abort/timeout race semantics preserved exactly
Medium	Testing	New code has corresponding tests	Pass	Readiness suite covers rewritten paths; waitForReady returns a Promise assertion still holds
Medium	Testing	Error paths and edge cases tested	Pass	Timeout/graceful-degradation cases covered; suggested adding a thenable assertion for the disabled preset early-return
Medium	Testing	Existing tests still pass (no regressions)	Pass	Full @percy/dom karma suite run at head SHA: 902 tests, 0 failures (Chrome + Firefox)
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	No data fetching
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable
Medium	Quality	Follows existing codebase patterns	Pass	Promise-chain style matches the rest of dom src; ESLint guard verified working on repo's ESLint 7.32 config cascade
Medium	Quality	Changes are focused (single concern)	Pass	Two files, one concern (PER-10121)
Low	Quality	Meaningful names, no dead code	Pass	—
Low	Quality	Comments explain why, not what	Pass	New comments document the browser-injection contract and ticket
Low	Quality	No unnecessary dependencies added	Pass	No dependency changes; .eslintrc not in published files

Findings

• File: packages/dom/src/readiness.js:529

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: Sync throws now escape waitForReady instead of landing in result.error. Previously waitForReady was async, so a synchronous throw during check setup became a rejection captured by the catch. Now runAllChecks runs synchronously before .catch attaches, so a sync throw (e.g. in checkFontReady, whose body executes outside a Promise executor) propagates out as an exception. All in-repo callers (packages/core/src/page.js:290-296, packages/sdk-utils/src/serialize-dom.js:59-66,113-119) are protected, but third-party SDK code calling PercyDOM.waitForReady(cfg).then(...) directly would see an escaped exception instead of a captured error.

• Suggestion: Wrap the call async-free: new Promise(resolve => resolve(runAllChecks(config, result, aborted))).then(() => { settled = true; }) — converts sync throws into rejections, restoring exact async-function semantics.

• File: packages/dom/src/.eslintrc:1

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The guard protects source, not the shipped artifact. The real contract is "the built bundle is async-free"; a future bundled dependency, babel helper, or rollup-target change could reintroduce async into dist/bundle.js without any src/ lint violation.

• Suggestion: Add a small regression test (or CI step) that parses the built bundle with acorn (already in the dependency tree) and asserts zero async/generator nodes — it would have caught the original 1.31.14 regression.

• File: packages/dom/src/.eslintrc:16

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: Top-level for await (const x of y) (ForOfStatement[await=true]) slips through all four selectors when it appears at module top level (inside a function the :function[async=true] selector catches the container). Verified against ESLint 7.32.

• Suggestion: Add a fifth selector: ForOfStatement[await=true].

• File: packages/dom/src/.eslintrc:5

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: Extensionless .eslintrc relies on legacy cascading config, which ESLint 9 flat config removes — if the repo migrates, this guard stops applying silently with no error.

• Suggestion: Add a one-line note in the header comment so a flat-config migration doesn't silently drop the guard (the bundle-AST test from the previous finding would also backstop this).

---

Verdict: PASS — Fix is correct, minimal, and verified: rebuilt bundle is async/generator-free by AST inspection, all 902 dom tests pass at head, and the lint guard demonstrably fires on violations. All findings are Low severity and non-blocking; Finding 1 is a recommended one-line hardening.

[prklm10]

RUN_REGRESSION