From 216d03dae3e451bee7a3ae9db5ea5e27eb1d0932 Mon Sep 17 00:00:00 2001 From: Yanzhen Yu Date: Thu, 1 Nov 2018 11:36:25 +0800 Subject: [PATCH] Fix the regexp performance issue Also move the addHoverClass implementation into the rebuild stage. So if there is still some corner case we have not handled, it will only affect the replayer part of rrweb. --- src/rebuild.ts | 23 ++++++++++++++++++++++- src/snapshot.ts | 31 ++----------------------------- src/types.ts | 1 + test/integration.ts | 1 + test/rebuild.test.ts | 43 +++++++++++++++++++++++++++++++++++++++++++ test/snapshot.test.ts | 42 +----------------------------------------- 6 files changed, 70 insertions(+), 71 deletions(-) create mode 100644 test/rebuild.test.ts diff --git a/src/rebuild.ts b/src/rebuild.ts index 476ae251..e4e79b28 100644 --- a/src/rebuild.ts +++ b/src/rebuild.ts @@ -115,6 +115,22 @@ function getTagName(n: elementNode): string { return tagName; } +const CSS_SELECTOR = /([^\r\n,{}]+)(,(?=[^}]*{)|\s*{)/g; +const HOVER_SELECTOR = /([^\\]):hover/g; +export function addHoverClass(cssText: string): string { + return cssText.replace(CSS_SELECTOR, (match, p1: string, p2: string) => { + if (HOVER_SELECTOR.test(p1)) { + const newSelector = p1.replace(HOVER_SELECTOR, '$1.\\:hover'); + return `${p1.replace(/\s*$/, '')}, ${newSelector.replace( + /^\s*/, + '', + )}${p2}`; + } else { + return match; + } + }); +} + function buildNode(n: serializedNodeWithId, doc: Document): Node | null { switch (n.type) { case NodeType.Document: @@ -139,6 +155,9 @@ function buildNode(n: serializedNodeWithId, doc: Document): Node | null { value = typeof value === 'boolean' ? '' : value; const isTextarea = tagName === 'textarea' && name === 'value'; const isRemoteCss = tagName === 'style' && name === '_cssText'; + if (isRemoteCss) { + value = addHoverClass(value); + } if (isTextarea || isRemoteCss) { const child = doc.createTextNode(value); node.appendChild(child); @@ -152,7 +171,9 @@ function buildNode(n: serializedNodeWithId, doc: Document): Node | null { } return node; case NodeType.Text: - return doc.createTextNode(n.textContent); + return doc.createTextNode( + n.isStyle ? addHoverClass(n.textContent) : n.textContent, + ); case NodeType.CDATA: return doc.createCDATASection(n.textContent); case NodeType.Comment: diff --git a/src/snapshot.ts b/src/snapshot.ts index bb4ed167..f1376ae7 100644 --- a/src/snapshot.ts +++ b/src/snapshot.ts @@ -17,36 +17,11 @@ export function resetId() { _id = 1; } -const CSS_RULE = /(([#|\.]{0,1}[a-z0-9\[\]=:]+[\s|,]*)+)\s(\{[\s\S]?[^}]*})/; -const CSS_RULE_GLOBAL = /(([#|\.]{0,1}[a-z0-9\[\]=:]+[\s|,]*)+)\s(\{[\s\S]?[^}]*})/g; -const HOVER_SELECTOR = /([^\\]):hover/g; -export function addHoverClass(cssText: string): string { - const matches = cssText.match(CSS_RULE_GLOBAL) || []; - for (const match of matches) { - const [, selectorText = '', , rules = ''] = match.match(CSS_RULE) || []; - const selectors = selectorText - .split(',') - .map(selector => selector.trim()) - .map(selector => { - if (HOVER_SELECTOR.test(selector)) { - const newSelector = selector.replace(HOVER_SELECTOR, '$1.\\:hover'); - selector += `, ${newSelector}`; - } - return selector; - }); - cssText = cssText.replace(match, selectors.join(', ') + ' ' + rules); - } - return cssText; -} - function getCssRulesString(s: CSSStyleSheet): string | null { try { const rules = s.rules || s.cssRules; return rules - ? Array.from(rules).reduce( - (prev, cur) => (prev += addHoverClass(cur.cssText)), - '', - ) + ? Array.from(rules).reduce((prev, cur) => (prev += cur.cssText), '') : null; } catch (error) { return null; @@ -177,12 +152,10 @@ function serializeNode(n: Node, doc: Document): serializedNode | false { if (parentTagName === 'SCRIPT') { textContent = 'SCRIPT_PLACEHOLDER'; } - if (parentTagName === 'STYLE') { - textContent = addHoverClass(textContent || ''); - } return { type: NodeType.Text, textContent: textContent || '', + isStyle: parentTagName === 'STYLE' ? true : undefined, }; case n.CDATA_SECTION_NODE: return { diff --git a/src/types.ts b/src/types.ts index 6fbe58e2..62d6af8b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -32,6 +32,7 @@ export type elementNode = { export type textNode = { type: NodeType.Text; textContent: string; + isStyle?: true; }; export type cdataNode = { diff --git a/test/integration.ts b/test/integration.ts index 8c71101f..f668b625 100644 --- a/test/integration.ts +++ b/test/integration.ts @@ -70,6 +70,7 @@ describe('integration tests', () => { before(async () => { this.server = await server(); this.browser = await puppeteer.launch({ + executablePath: '/home/yanzhen/Desktop/chrome-linux/chrome', // headless: false, }); diff --git a/test/rebuild.test.ts b/test/rebuild.test.ts new file mode 100644 index 00000000..d93bf64a --- /dev/null +++ b/test/rebuild.test.ts @@ -0,0 +1,43 @@ +import 'mocha'; +import { expect } from 'chai'; +import { addHoverClass } from '../src/rebuild'; + +describe('add hover class to hover selector related rules', () => { + it('will do nothing to css text without :hover', () => { + const cssText = 'body { color: white }'; + expect(addHoverClass(cssText)).to.equal(cssText); + }); + + it('can add hover class to css text', () => { + const cssText = '.a:hover { color: white }'; + expect(addHoverClass(cssText)).to.equal( + '.a:hover, .a.\\:hover { color: white }', + ); + }); + + it('can add hover class when there is multi selector', () => { + const cssText = '.a, .b:hover, .c { color: white }'; + expect(addHoverClass(cssText)).to.equal( + '.a, .b:hover, .b.\\:hover, .c { color: white }', + ); + }); + + it('can add hover class when :hover is not the end of selector', () => { + const cssText = 'div:hover::after { color: white }'; + expect(addHoverClass(cssText)).to.equal( + 'div:hover::after, div.\\:hover::after { color: white }', + ); + }); + + it('can add hover class when the selector has multi :hover', () => { + const cssText = 'a:hover b:hover { color: white }'; + expect(addHoverClass(cssText)).to.equal( + 'a:hover b:hover, a.\\:hover b.\\:hover { color: white }', + ); + }); + + it('will ignore :hover in css value', () => { + const cssText = '.a::after { content: ":hover" }'; + expect(addHoverClass(cssText)).to.equal(cssText); + }); +}); diff --git a/test/snapshot.test.ts b/test/snapshot.test.ts index c5e12226..0720f120 100644 --- a/test/snapshot.test.ts +++ b/test/snapshot.test.ts @@ -1,6 +1,6 @@ import 'mocha'; import { expect } from 'chai'; -import { absoluteToStylesheet, addHoverClass } from '../src/snapshot'; +import { absoluteToStylesheet } from '../src/snapshot'; describe('absolute url to stylesheet', () => { const href = 'http://localhost/css/style.css'; @@ -29,43 +29,3 @@ describe('absolute url to stylesheet', () => { ).to.equal(`url('http://localhost/a.jpg')`); }); }); - -describe('add hover class to hover selector related rules', () => { - it('will do nothing to css text without :hover', () => { - const cssText = 'body { color: white }'; - expect(addHoverClass(cssText)).to.equal(cssText); - }); - - it('can add hover class to css text', () => { - const cssText = '.a:hover { color: white }'; - expect(addHoverClass(cssText)).to.equal( - '.a:hover, .a.\\:hover { color: white }', - ); - }); - - it('can add hover class when there is multi selector', () => { - const cssText = '.a, .b:hover, .c { color: white }'; - expect(addHoverClass(cssText)).to.equal( - '.a, .b:hover, .b.\\:hover, .c { color: white }', - ); - }); - - it('can add hover class when :hover is not the end of selector', () => { - const cssText = 'div:hover::after { color: white }'; - expect(addHoverClass(cssText)).to.equal( - 'div:hover::after, div.\\:hover::after { color: white }', - ); - }); - - it('can add hover class when the selector has multi :hover', () => { - const cssText = 'a:hover b:hover { color: white }'; - expect(addHoverClass(cssText)).to.equal( - 'a:hover b:hover, a.\\:hover b.\\:hover { color: white }', - ); - }); - - it('will ignore :hover in css value', () => { - const cssText = '.a::after { content: ":hover" }'; - expect(addHoverClass(cssText)).to.equal(cssText); - }); -});