-
Notifications
You must be signed in to change notification settings - Fork 56
PER 7292 #2177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
PER 7292 #2177
Changes from all commits
7f9d847
faf5789
a7cb982
32cb384
42cb9b8
30224bc
14c5de7
7cbb2b3
cab957e
32f5fce
f25a181
25b87aa
88ad0bf
6d4b313
4d75f34
297afa2
2846dfd
d0d36f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import fs from 'fs'; | ||
| import path from 'path'; | ||
| import logger from '@percy/logger'; | ||
| import Network from './network.js'; | ||
| import { PERCY_DOM } from './api.js'; | ||
|
|
@@ -9,6 +10,31 @@ import { | |
| serializeFunction | ||
| } from './utils.js'; | ||
|
|
||
| // Cached preflight script for closed shadow root and ElementInternals interception | ||
| let _preflightScript = null; | ||
| async function getPreflightScript() { | ||
| if (!_preflightScript) { | ||
| let pkgRoot = path.resolve(path.dirname(PERCY_DOM), '..'); | ||
| let candidates = [ | ||
| path.join(pkgRoot, 'src', 'preflight.js'), | ||
| path.join(pkgRoot, 'dist', 'preflight.js'), | ||
| path.join(path.dirname(PERCY_DOM), 'preflight.js') | ||
| ]; | ||
| for (let candidate of candidates) { | ||
| try { | ||
| _preflightScript = await fs.promises.readFile(candidate, 'utf-8'); | ||
| break; | ||
| } catch { | ||
| // try next candidate | ||
| } | ||
| } | ||
| if (!_preflightScript) { | ||
| _preflightScript = ''; // graceful fallback if file not found in any location | ||
| } | ||
| } | ||
| return _preflightScript; | ||
| } | ||
|
|
||
| export class Page { | ||
| static TIMEOUT = undefined; | ||
|
|
||
|
|
@@ -187,7 +213,7 @@ export class Page { | |
| execute, | ||
| ...snapshot | ||
| }) { | ||
| let { name, width, enableJavaScript, disableShadowDOM, forceShadowAsLightDOM, domTransformation, reshuffleInvalidTags, ignoreCanvasSerializationErrors, ignoreStyleSheetSerializationErrors, pseudoClassEnabledElements } = snapshot; | ||
| let { name, width, enableJavaScript, disableShadowDOM, forceShadowAsLightDOM, domTransformation, reshuffleInvalidTags, ignoreCanvasSerializationErrors, ignoreStyleSheetSerializationErrors, ignoreIframeSelectors, pseudoClassEnabledElements } = snapshot; | ||
| this.log.debug(`Taking snapshot: ${name}${width ? ` @${width}px` : ''}`, this.meta); | ||
|
|
||
| // wait for any specified timeout | ||
|
|
@@ -211,6 +237,21 @@ export class Page { | |
| // wait for any final network activity before capturing the dom snapshot | ||
| await this.network.idle(); | ||
|
|
||
| // wait for custom elements to be defined before capturing | ||
| /* istanbul ignore next: no instrumenting injected code */ | ||
| await this.eval(function() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds up to a 5-second hard wait to every snapshot whenever the page has a single This will regress P50 build latency for SDK consumers.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Reduced timeout from 5000ms to 500ms. |
||
| let undefinedEls = document.querySelectorAll(':not(:defined)'); | ||
| if (!undefinedEls.length) return Promise.resolve(); | ||
| return Promise.race([ | ||
| Promise.all( | ||
| Array.from(undefinedEls).map(function(el) { | ||
| return window.customElements.whenDefined(el.localName); | ||
| }) | ||
| ), | ||
| new Promise(function(r) { setTimeout(r, 500); }) | ||
| ]); | ||
| }); | ||
|
|
||
| await this.insertPercyDom(); | ||
|
|
||
| // serialize and capture a DOM snapshot | ||
|
|
@@ -221,7 +262,7 @@ export class Page { | |
| /* eslint-disable-next-line no-undef */ | ||
| domSnapshot: PercyDOM.serialize(options), | ||
| url: document.URL | ||
| }), { enableJavaScript, disableShadowDOM, forceShadowAsLightDOM, domTransformation, reshuffleInvalidTags, ignoreCanvasSerializationErrors, ignoreStyleSheetSerializationErrors, pseudoClassEnabledElements }); | ||
| }), { enableJavaScript, disableShadowDOM, forceShadowAsLightDOM, domTransformation, reshuffleInvalidTags, ignoreCanvasSerializationErrors, ignoreStyleSheetSerializationErrors, ignoreIframeSelectors, pseudoClassEnabledElements }); | ||
|
|
||
| return { ...snapshot, ...capture }; | ||
| } | ||
|
|
@@ -238,8 +279,22 @@ export class Page { | |
| if (session.isDocument) { | ||
| session.on('Target.attachedToTarget', this._handleAttachedToTarget); | ||
|
|
||
| // Chain preflight injection after Page.enable to ensure the Page domain | ||
| // is ready before addScriptToEvaluateOnNewDocument | ||
| let pageEnablePromise = session.send('Page.enable'); | ||
| commands.push( | ||
| session.send('Page.enable'), | ||
| pageEnablePromise.then(() => { | ||
| return getPreflightScript().then(script => { | ||
| if (script) { | ||
| return session.send('Page.addScriptToEvaluateOnNewDocument', { source: script }) | ||
| .catch(err => { | ||
| if (!err.message?.includes('closed') && !err.message?.includes('destroyed')) { | ||
| logger('core:page').debug('Preflight script injection failed:', err.message); | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
| }), | ||
| session.send('Page.setLifecycleEventsEnabled', { enabled: true }), | ||
| session.send('Security.setIgnoreCertificateErrors', { ignore: true }), | ||
| session.send('Emulation.setScriptExecutionDisabled', { value: !this.enableJavaScript }), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // Percy Pre-flight Script | ||
| // Injected before page scripts to intercept closed shadow roots and ElementInternals. | ||
| // This enables Percy to capture content inside closed shadow DOM and custom element states. | ||
|
|
||
| (function() { | ||
| if (window.__percyPreflightActive) return; | ||
| window.__percyPreflightActive = true; | ||
|
|
||
| // --- Intercept closed shadow roots --- | ||
| let closedShadowRoots = new WeakMap(); | ||
| let origAttachShadow = window.Element.prototype.attachShadow; | ||
| window.Element.prototype.attachShadow = function(init) { | ||
| let root = origAttachShadow.apply(this, arguments); | ||
| if (init?.mode === 'closed') { | ||
| closedShadowRoots.set(this, root); | ||
| } | ||
| return root; | ||
| }; | ||
| window.__percyClosedShadowRoots = closedShadowRoots; | ||
|
|
||
| // --- Intercept ElementInternals for :state() capture --- | ||
| if (typeof window.HTMLElement.prototype.attachInternals === 'function') { | ||
| let internalsMap = new WeakMap(); | ||
| let origAttachInternals = window.HTMLElement.prototype.attachInternals; | ||
| window.HTMLElement.prototype.attachInternals = function() { | ||
| let internals = origAttachInternals.apply(this, arguments); | ||
| internalsMap.set(this, internals); | ||
| return internals; | ||
| }; | ||
| window.__percyInternals = internalsMap; | ||
| } | ||
| })(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading
domSnapshot?.[0]?.fidelityRegionssuggests multi-root DOMs are expected, but onlydomSnapshot[0]is consulted — fidelity regions captured by subsequent roots are silently dropped. If responsive snapshot capture produces multiple domSnapshots (per-width), you'll be under-reporting. Either merge regions across all entries or add a comment explaining why only the first matters.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment explaining that only the first domSnapshot's fidelity regions are used because for responsive captures with multiple widths, regions are width-independent (same DOM structure) so the first entry is representative.