fix: always set percy-data attributes when serializing #1450
Conversation
| cache[platform].$ = parseDOM(serializeDOM(), platform); | ||
| // save DOM, only after serializing it | ||
| // clone DOM, so that during next platform iteration, serializeDOM doesn't change current original DOM | ||
| cache[platform].dom = platform === 'shadow' ? dom : dom.cloneNode(true); |
There was a problem hiding this comment.
why are we cloning DOM here?
because our assertions are againts original DOM, if we clone it we're not checking against original DOM when APIs change in future and there is drift between cloned DOM and original DOM.
There was a problem hiding this comment.
serializeDOM() already does that for us.
There was a problem hiding this comment.
Here is the test preparation workflow that is happening for Chrome -
- We create
plainDOM, and then runserialzeDOMon it, and store this incache['plain'] - Then we take the same DOM attach shadow components to it, and then run
serializeDOMon it again, and store this incache['shadow'] - So here, the
plainDOM part is getting serialized twice, and since while storing incache['plain']we store by reference, it get's updated to be the newly serialized plain DOM, whereas the serializedDOM from first iteration is intact.
There was a problem hiding this comment.
understood, in this case we shouldn't be calling dom.cloneNode rather our custom clone node traversal code, will update and merge.
This comment was marked as outdated.
This comment was marked as outdated.
itsjwala
added
🐛 bug
|
This PR is stale because it has been open for more than 14 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
|
This PR was closed because it has been stalled for 28 days with no activity. |
context-
data-percy-element-idis already available, if yes we do not add a new one.data-percy-element-idattribute is used to create url's for serialized canvas.data-percy-element-idfor canvas as -abcddata-percy-element-idtherefore, we do not assign a new one.abcdfor thisabcdwhich was derived fromdata-percy-element-idwhich we have not updated when seriazing 500px dome.related pr's-