From 3361449964f0349c2dcd8e53617708c32a6b7475 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 2 Sep 2022 01:41:07 +0200 Subject: [PATCH] perf: hoist the browser detection condition --- src/lib/stylesheet.js | 26 ++++++++----------- src/stylesheet-registry.js | 10 ++----- test/helpers/with-mock.js | 6 ++--- test/stylesheet-registry.js | 47 +++++++++++++++++++++++++++++---- test/stylesheet.js | 52 ++++++++++++++++++++++++++++++++++--- 5 files changed, 107 insertions(+), 34 deletions(-) diff --git a/src/lib/stylesheet.js b/src/lib/stylesheet.js index 38eec882..c78157cc 100644 --- a/src/lib/stylesheet.js +++ b/src/lib/stylesheet.js @@ -10,11 +10,7 @@ const isProd = const isString = o => Object.prototype.toString.call(o) === '[object String]' export default class StyleSheet { - constructor({ - name = 'stylesheet', - optimizeForSpeed = isProd, - isBrowser = typeof window !== 'undefined' - } = {}) { + constructor({ name = 'stylesheet', optimizeForSpeed = isProd } = {}) { invariant(isString(name), '`name` must be a string') this._name = name this._deletedRulePlaceholder = `#${name}-deleted-rule____{}` @@ -24,15 +20,14 @@ export default class StyleSheet { '`optimizeForSpeed` must be a boolean' ) this._optimizeForSpeed = optimizeForSpeed - this._isBrowser = isBrowser - this._serverSheet = undefined this._tags = [] this._injected = false this._rulesCount = 0 const node = - this._isBrowser && document.querySelector('meta[property="csp-nonce"]') + typeof window !== 'undefined' && + document.querySelector('meta[property="csp-nonce"]') this._nonce = node ? node.getAttribute('content') : null } @@ -58,7 +53,7 @@ export default class StyleSheet { inject() { invariant(!this._injected, 'sheet already injected') this._injected = true - if (this._isBrowser && this._optimizeForSpeed) { + if (typeof window !== 'undefined' && this._optimizeForSpeed) { this._tags[0] = this.makeStyleTag(this._name) this._optimizeForSpeed = 'insertRule' in this.getSheet() if (!this._optimizeForSpeed) { @@ -112,7 +107,7 @@ export default class StyleSheet { insertRule(rule, index) { invariant(isString(rule), '`insertRule` accepts only strings') - if (!this._isBrowser) { + if (typeof window === 'undefined') { if (typeof index !== 'number') { index = this._serverSheet.cssRules.length } @@ -149,8 +144,9 @@ export default class StyleSheet { } replaceRule(index, rule) { - if (this._optimizeForSpeed || !this._isBrowser) { - const sheet = this._isBrowser ? this.getSheet() : this._serverSheet + if (this._optimizeForSpeed || typeof window === 'undefined') { + const sheet = + typeof window !== 'undefined' ? this.getSheet() : this._serverSheet if (!rule.trim()) { rule = this._deletedRulePlaceholder } @@ -184,7 +180,7 @@ export default class StyleSheet { } deleteRule(index) { - if (!this._isBrowser) { + if (typeof window === 'undefined') { this._serverSheet.deleteRule(index) return } @@ -202,7 +198,7 @@ export default class StyleSheet { flush() { this._injected = false this._rulesCount = 0 - if (this._isBrowser) { + if (typeof window !== 'undefined') { this._tags.forEach(tag => tag && tag.parentNode.removeChild(tag)) this._tags = [] } else { @@ -212,7 +208,7 @@ export default class StyleSheet { } cssRules() { - if (!this._isBrowser) { + if (typeof window === 'undefined') { return this._serverSheet.cssRules } diff --git a/src/stylesheet-registry.js b/src/stylesheet-registry.js index d887265d..7102ccb4 100644 --- a/src/stylesheet-registry.js +++ b/src/stylesheet-registry.js @@ -19,11 +19,7 @@ function mapRulesToStyle(cssRules, options = {}) { }) } export class StyleSheetRegistry { - constructor({ - styleSheet = null, - optimizeForSpeed = false, - isBrowser = typeof window !== 'undefined' - } = {}) { + constructor({ styleSheet = null, optimizeForSpeed = false } = {}) { this._sheet = styleSheet || new DefaultStyleSheet({ @@ -37,8 +33,6 @@ export class StyleSheetRegistry { this._optimizeForSpeed = this._sheet.isOptimizeForSpeed() } - this._isBrowser = isBrowser - this._fromServer = undefined this._indices = {} this._instancesCounts = {} @@ -51,7 +45,7 @@ export class StyleSheetRegistry { this._optimizeForSpeed = this._sheet.isOptimizeForSpeed() } - if (this._isBrowser && !this._fromServer) { + if (typeof window !== 'undefined' && !this._fromServer) { this._fromServer = this.selectFromServer() this._instancesCounts = Object.keys(this._fromServer).reduce( (acc, tagName) => { diff --git a/test/helpers/with-mock.js b/test/helpers/with-mock.js index f72fd0b5..159d1b46 100644 --- a/test/helpers/with-mock.js +++ b/test/helpers/with-mock.js @@ -11,9 +11,9 @@ export default function withMock(mockFn, testFn) { } export function withMockDocument(t) { - const originalDocument = global.document + const originalDocument = globalThis.document // We need to stub a document in order to simulate the meta tag - global.document = { + globalThis.document = { querySelector(query) { t.is(query, 'meta[property="csp-nonce"]') return { @@ -26,6 +26,6 @@ export function withMockDocument(t) { } return () => { - global.document = originalDocument + globalThis.document = originalDocument } } diff --git a/test/stylesheet-registry.js b/test/stylesheet-registry.js index 3c9b3b50..0f3a9727 100644 --- a/test/stylesheet-registry.js +++ b/test/stylesheet-registry.js @@ -7,7 +7,7 @@ import makeSheet, { invalidRules } from './stylesheet' import withMock, { withMockDocument } from './helpers/with-mock' import { computeId, computeSelector } from '../src/lib/hash' -function makeRegistry(options = { optimizeForSpeed: true, isBrowser: true }) { +function makeRegistry(options = { optimizeForSpeed: true }) { const registry = new StyleSheetRegistry({ styleSheet: makeSheet(options), ...options @@ -32,7 +32,12 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const registry = makeRegistry(options) + registry.add({ id: '123', children: options.optimizeForSpeed ? [cssRule] : cssRule @@ -68,6 +73,10 @@ test( ['jsx-456', 'div { color: red }p { color: red }'] ]) } + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) @@ -75,6 +84,8 @@ test( test( 'add - filters out invalid rules (index `-1`)', withMock(withMockDocument, t => { + globalThis.window = globalThis + const registry = makeRegistry() // Insert a valid rule @@ -90,12 +101,16 @@ test( ['jsx-123', 'div { color: red }'], ['jsx-678', 'div { color: red }'] ]) + + delete globalThis.window }) ) test( 'it does not throw when inserting an invalid rule', withMock(withMockDocument, t => { + globalThis.window = globalThis + const registry = makeRegistry() // Insert a valid rule @@ -107,11 +122,13 @@ test( }) t.deepEqual(registry.cssRules(), [['jsx-123', 'div { color: red }']]) + + delete globalThis.window }) ) test('add - sanitizes dynamic CSS on the server', t => { - const registry = makeRegistry({ optimizeForSpeed: false, isBrowser: false }) + const registry = makeRegistry({ optimizeForSpeed: false }) registry.add({ id: '123', @@ -130,9 +147,9 @@ test('add - sanitizes dynamic CSS on the server', t => { }) test('add - nonce is properly fetched from meta tag', t => { - const originalDocument = global.document + const originalDocument = globalThis.document // We need to stub a document in order to simulate the meta tag - global.document = { + globalThis.document = { querySelector(query) { t.is(query, 'meta[property="csp-nonce"]') return { @@ -144,12 +161,16 @@ test('add - nonce is properly fetched from meta tag', t => { } } + globalThis.window = globalThis + const registry = makeRegistry() registry.add({ id: '123', children: [cssRule] }) t.is(registry._sheet._nonce, 'test-nonce') - global.document = originalDocument + globalThis.document = originalDocument + + delete globalThis.window }) // registry.remove @@ -165,6 +186,10 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const registry = makeRegistry(options) registry.add({ id: '123', @@ -192,6 +217,10 @@ test( registry.remove({ id: '345' }) // now the registry should be empty t.deepEqual(registry.cssRules(), []) + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) @@ -209,6 +238,10 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const registry = makeRegistry(options) registry.add({ id: '123', @@ -246,6 +279,10 @@ test( ) // 123 replaced with 345 t.deepEqual(registry.cssRules(), [['jsx-345', cssRuleAlt]]) + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) diff --git a/test/stylesheet.js b/test/stylesheet.js index fc0947ef..f5d1aee7 100644 --- a/test/stylesheet.js +++ b/test/stylesheet.js @@ -7,9 +7,7 @@ import withMock, { withMockDocument } from './helpers/with-mock' export const invalidRules = ['invalid rule'] -export default function makeSheet( - options = { optimizeForSpeed: true, isBrowser: true } -) { +export default function makeSheet(options = { optimizeForSpeed: true }) { const sheet = new StyleSheet(options) // mocks sheet.makeStyleTag = function(name, cssString) { @@ -69,6 +67,8 @@ export default function makeSheet( test( 'can change optimizeForSpeed only when the stylesheet is empty', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() @@ -85,6 +85,8 @@ test( t.notThrows(() => { sheet.setOptimizeForSpeed(false) }) + + delete globalThis.window }) ) @@ -100,6 +102,10 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const sheet = makeSheet(options) sheet.inject() @@ -111,6 +117,10 @@ test( { cssText: 'div { color: red }' }, { cssText: 'div { color: green }' } ]) + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) @@ -118,6 +128,8 @@ test( test( 'insertRule - returns the rule index', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() @@ -126,23 +138,31 @@ test( i = sheet.insertRule('div { color: red }') t.is(i, 1) + + delete globalThis.window }) ) test( 'insertRule - handles invalid rules and returns -1 as index', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() const i = sheet.insertRule(invalidRules[0]) t.is(i, -1) + + delete globalThis.window }) ) test( 'insertRule - does not fail when the css is a String object', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() @@ -152,6 +172,8 @@ test( t.deepEqual(sheet.cssRules(), [ { cssText: new String('div { color: red }') } ]) + + delete globalThis.window /* eslint-enable */ }) ) @@ -169,6 +191,10 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const sheet = makeSheet(options) sheet.inject() @@ -181,6 +207,10 @@ test( t.is(sheet.length, rulesCount) t.deepEqual(sheet.cssRules(), [{ cssText: 'div { color: red }' }, null]) + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) @@ -188,12 +218,16 @@ test( test( 'deleteRule - does not throw when the rule at index does not exist', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() t.notThrows(() => { sheet.deleteRule(sheet.length + 1) }) + + delete globalThis.window }) ) @@ -210,6 +244,10 @@ test( ] options.forEach(options => { + if (options.isBrowser) { + globalThis.window = globalThis + } + const sheet = makeSheet(options) sheet.inject() @@ -217,6 +255,10 @@ test( sheet.replaceRule(index, 'p { color: hotpink }') t.deepEqual(sheet.cssRules(), [{ cssText: 'p { color: hotpink }' }]) + + if (options.isBrowser) { + delete globalThis.window + } }) }) ) @@ -224,6 +266,8 @@ test( test( 'replaceRule - handles invalid rules gracefully', withMock(withMockDocument, t => { + globalThis.window = globalThis + const sheet = makeSheet() sheet.inject() @@ -240,5 +284,7 @@ test( // therefore the lib must insert a delete placeholder which resolves to `null` // when `cssRules()` is called. t.deepEqual(sheet.cssRules(), [{ cssText: 'div { color: red }' }, null]) + + delete globalThis.window }) )