Skip to content

Commit f4537e6

Browse files
author
jaj1014
committed
fix: inserted styles lost when moving elements
fix code for nodejs tests change fix direction to avoid issues with duplicate styles format issues swap waitForTimeout for waitForRAF in test that flaked Add unit tests for new functions Fix broken test causes by file formatting removing spaced
1 parent e607e83 commit f4537e6

File tree

4 files changed

+358
-11
lines changed

4 files changed

+358
-11
lines changed

packages/rrdom/src/diff.ts

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ function diffChildren(
379379
nodeMatching(oldStartNode, newEndNode, replayer.mirror, rrnodeMirror)
380380
) {
381381
try {
382-
oldTree.insertBefore(oldStartNode, oldEndNode.nextSibling);
382+
handleInsertBefore(oldTree, oldStartNode, oldEndNode.nextSibling);
383383
} catch (e) {
384384
console.warn(e);
385385
}
@@ -390,7 +390,7 @@ function diffChildren(
390390
nodeMatching(oldEndNode, newStartNode, replayer.mirror, rrnodeMirror)
391391
) {
392392
try {
393-
oldTree.insertBefore(oldEndNode, oldStartNode);
393+
handleInsertBefore(oldTree, oldEndNode, oldStartNode);
394394
} catch (e) {
395395
console.warn(e);
396396
}
@@ -415,7 +415,7 @@ function diffChildren(
415415
nodeMatching(nodeToMove, newStartNode, replayer.mirror, rrnodeMirror)
416416
) {
417417
try {
418-
oldTree.insertBefore(nodeToMove, oldStartNode);
418+
handleInsertBefore(oldTree, nodeToMove, oldStartNode);
419419
} catch (e) {
420420
console.warn(e);
421421
}
@@ -449,7 +449,7 @@ function diffChildren(
449449
}
450450

451451
try {
452-
oldTree.insertBefore(newNode, oldStartNode || null);
452+
handleInsertBefore(oldTree, newNode, oldStartNode || null);
453453
} catch (e) {
454454
console.warn(e);
455455
}
@@ -471,7 +471,7 @@ function diffChildren(
471471
rrnodeMirror,
472472
);
473473
try {
474-
oldTree.insertBefore(newNode, referenceNode);
474+
handleInsertBefore(oldTree, newNode, referenceNode);
475475
} catch (e) {
476476
console.warn(e);
477477
}
@@ -579,3 +579,64 @@ export function nodeMatching(
579579
if (node1Id === -1 || node1Id !== node2Id) return false;
580580
return sameNodeType(node1, node2);
581581
}
582+
583+
/**
584+
* Copies CSSRules and their position from HTML style element which don't exist in it's innerText
585+
*/
586+
function getInsertedStylesFromElement(
587+
styleElement: HTMLStyleElement,
588+
): Array<{ index: number; cssRuleText: string }> | undefined {
589+
const elementCssRules = styleElement.sheet?.cssRules;
590+
if (!elementCssRules || !elementCssRules.length) return;
591+
// style sheet w/ innerText styles to diff with actual and get only inserted styles
592+
const tempStyleSheet = new CSSStyleSheet();
593+
tempStyleSheet.replaceSync(styleElement.innerText);
594+
595+
const innerTextStylesMap: { [key: string]: CSSRule } = {};
596+
597+
for (let i = 0; i < tempStyleSheet.cssRules.length; i++) {
598+
innerTextStylesMap[tempStyleSheet.cssRules[i].cssText] =
599+
tempStyleSheet.cssRules[i];
600+
}
601+
602+
const insertedStylesStyleSheet = [];
603+
604+
for (let i = 0; i < elementCssRules?.length; i++) {
605+
const cssRuleText = elementCssRules[i].cssText;
606+
607+
if (!innerTextStylesMap[cssRuleText]) {
608+
insertedStylesStyleSheet.push({
609+
index: i,
610+
cssRuleText,
611+
});
612+
}
613+
}
614+
615+
return insertedStylesStyleSheet;
616+
}
617+
618+
/**
619+
* Conditionally copy insertedStyles for STYLE nodes and apply after calling insertBefore'
620+
* For non-STYLE nodes, just insertBefore
621+
*/
622+
export function handleInsertBefore(
623+
oldTree: Node,
624+
nodeToMove: Node,
625+
insertBeforeNode: Node | null,
626+
): void {
627+
let insertedStyles;
628+
629+
if (nodeToMove.nodeName === 'STYLE') {
630+
insertedStyles = getInsertedStylesFromElement(
631+
nodeToMove as HTMLStyleElement,
632+
);
633+
}
634+
635+
oldTree.insertBefore(nodeToMove, insertBeforeNode);
636+
637+
if (insertedStyles && insertedStyles.length) {
638+
insertedStyles.forEach(({ cssRuleText, index }) => {
639+
(nodeToMove as HTMLStyleElement).sheet?.insertRule(cssRuleText, index);
640+
});
641+
}
642+
}

packages/rrdom/test/diff.test.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
ReplayerHandler,
2424
nodeMatching,
2525
sameNodeType,
26+
handleInsertBefore,
2627
} from '../src/diff';
2728
import type { IRRElement, IRRNode } from '../src/document';
2829
import { Replayer } from 'rrweb';
@@ -1441,7 +1442,7 @@ describe('diff algorithm for rrdom', () => {
14411442
const rrHtmlEl = rrDocument.createElement('html');
14421443
rrDocument.mirror.add(rrHtmlEl, rrdom.getDefaultSN(rrHtmlEl, ${htmlElId}));
14431444
rrIframeEl.contentDocument.appendChild(rrHtmlEl);
1444-
1445+
14451446
const replayer = {
14461447
mirror: rrdom.createMirror(),
14471448
applyCanvas: () => {},
@@ -1450,7 +1451,7 @@ describe('diff algorithm for rrdom', () => {
14501451
applyStyleSheetMutation: () => {},
14511452
};
14521453
rrdom.diff(iframeEl, rrIframeEl, replayer);
1453-
1454+
14541455
iframeEl.contentDocument.documentElement.className =
14551456
'${className.toLowerCase()}';
14561457
iframeEl.contentDocument.childNodes.length === 2 &&
@@ -1972,4 +1973,69 @@ describe('diff algorithm for rrdom', () => {
19721973
expect(nodeMatching(node1, node2, NodeMirror, rrdomMirror)).toBeFalsy();
19731974
});
19741975
});
1976+
1977+
describe('test handleInsertBefore function', () => {
1978+
it('should insert nodeToMove before insertBeforeNode in oldTree for non-style elements', () => {
1979+
const oldTree = document.createElement('div');
1980+
const nodeToMove = document.createElement('div');
1981+
const insertBeforeNode = document.createElement('div');
1982+
oldTree.appendChild(insertBeforeNode);
1983+
1984+
expect(oldTree.children.length).toEqual(1);
1985+
1986+
handleInsertBefore(oldTree, nodeToMove, insertBeforeNode);
1987+
1988+
expect(oldTree.children.length).toEqual(2);
1989+
expect(oldTree.children[0]).toEqual(nodeToMove);
1990+
});
1991+
1992+
it('should not drop inserted styles when moving a style element with inserted styles', async () => {
1993+
function MockCSSStyleSheet() {
1994+
this.replaceSync = jest.fn();
1995+
this.cssRules = [{ cssText: baseStyle }];
1996+
}
1997+
1998+
jest
1999+
.spyOn(window, 'CSSStyleSheet')
2000+
.mockImplementationOnce(MockCSSStyleSheet as any);
2001+
2002+
const baseStyle = 'body {margin: 0;}';
2003+
const insertedStyle = 'div {display: flex;}';
2004+
2005+
document.write('<html></html>');
2006+
2007+
const insertBeforeNode = document.createElement('style');
2008+
document.documentElement.appendChild(insertBeforeNode);
2009+
2010+
const nodeToMove = document.createElement('style');
2011+
nodeToMove.appendChild(document.createTextNode(baseStyle));
2012+
document.documentElement.appendChild(nodeToMove);
2013+
nodeToMove.sheet?.insertRule(insertedStyle);
2014+
2015+
// validate dom prior to moving element
2016+
expect(document.documentElement.children.length).toEqual(4);
2017+
expect(document.documentElement.children[2]).toEqual(insertBeforeNode);
2018+
expect(document.documentElement.children[3]).toEqual(nodeToMove);
2019+
expect(nodeToMove.sheet?.cssRules.length).toEqual(2);
2020+
expect(nodeToMove.sheet?.cssRules[0].cssText).toEqual(insertedStyle);
2021+
expect(nodeToMove.sheet?.cssRules[1].cssText).toEqual(baseStyle);
2022+
2023+
// move the node
2024+
handleInsertBefore(
2025+
document.documentElement,
2026+
nodeToMove,
2027+
insertBeforeNode
2028+
);
2029+
2030+
// nodeToMove was inserted before
2031+
expect(document.documentElement.children.length).toEqual(4);
2032+
expect(document.documentElement.children[2]).toEqual(nodeToMove);
2033+
expect(document.documentElement.children[3]).toEqual(insertBeforeNode);
2034+
// styles persisted on the moved element
2035+
// w/ document.documentElement.insertBefore(nodeToMove, insertBeforeNode) insertedStyle wouldn't be copied
2036+
expect(nodeToMove.sheet?.cssRules.length).toEqual(2);
2037+
expect(nodeToMove.sheet?.cssRules[0].cssText).toEqual(insertedStyle);
2038+
expect(nodeToMove.sheet?.cssRules[1].cssText).toEqual(baseStyle);
2039+
});
2040+
});
19752041
});

0 commit comments

Comments
 (0)