Skip to content

Commit

Permalink
✨ Allow domTransformation to be passed as string (#1186)
Browse files Browse the repository at this point in the history
  • Loading branch information
itsjwala authored Feb 20, 2023
1 parent 609ae53 commit 4bc6ad1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
6 changes: 5 additions & 1 deletion packages/dom/src/serialize-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,13 @@ export function serializeDOM(options) {

if (domTransformation) {
try {
// eslint-disable-next-line no-eval
if (typeof (domTransformation) === 'string') domTransformation = window.eval(domTransformation);
domTransformation(ctx.clone.documentElement);
} catch (err) {
console.error('Could not transform the dom:', err.message);
let errorMessage = `Could not transform the dom: ${err.message}`;
ctx.warnings.add(errorMessage);
console.error(errorMessage);
}
}

Expand Down
27 changes: 25 additions & 2 deletions packages/dom/test/serialize-dom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,29 @@ describe('serializeDOM', () => {
expect(document.querySelector('.delete-me').innerText).toBe('Delete me');
});

it('logs any errors and returns the serialized DOM', () => {
it('String: transforms the DOM without modifying the original DOM', () => {
let { html } = serializeDOM({
domTransformation: "(dom) => { dom.querySelector('.delete-me').remove(); }"
});

expect(html).not.toMatch('Delete me');
expect(document.querySelector('.delete-me').innerText).toBe('Delete me');
});

it('String: Logs error when function is not correct', () => {
let { html, warnings } = serializeDOM({
domTransformation: "(dom) => { dom.querySelector('.delete-me').delete(); }"
});

expect(html).toMatch('Delete me');
expect(console.error)
.toHaveBeenCalledOnceWith('Could not transform the dom: dom.querySelector(...).delete is not a function');

expect(warnings).toEqual(['Could not transform the dom: dom.querySelector(...).delete is not a function']);
});

it('logs any errors and returns the serialized DOM', () => {
let { html, warnings } = serializeDOM({
domTransformation(dom) {
throw new Error('test error');
// eslint-disable-next-line no-unreachable
Expand All @@ -247,7 +268,9 @@ describe('serializeDOM', () => {

expect(html).toMatch('Delete me');
expect(console.error)
.toHaveBeenCalledOnceWith('Could not transform the dom:', 'test error');
.toHaveBeenCalledOnceWith('Could not transform the dom: test error');

expect(warnings).toEqual(['Could not transform the dom: test error']);
});
});
});

3 comments on commit 4bc6ad1

@slokerqe
Copy link

@slokerqe slokerqe commented on 4bc6ad1 Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@percy/cli -> 1.20.0
receiving this message:
2023-02-21T05:53:24.3702098Z [percy] Invalid snapshot options:
2023-02-21T05:53:24.3702791Z [percy] - domSnapshot: missing required property
2023-02-21T05:53:25.1644208Z [percy] Snapshot taken: <my_custom_snapshot_name>

After execution, Percy's build includes empty snapshots

@itsjwala
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slokerqe

could you open a Github discussion with the necessary information to debug further?

  • A reproducible example/percy config you're using, entire debug logs, SDK you're using.

@slokerqe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Please sign in to comment.