From e7ae1795e6979c83dba9318aeec672675f818486 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 13 Jun 2017 15:06:39 -0700 Subject: [PATCH 1/8] Minify combined class names --- src/exports.js | 1 - src/inject.js | 5 ++++- tests/inject_test.js | 26 +++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/exports.js b/src/exports.js index 2dadd66..ae40f17 100644 --- a/src/exports.js +++ b/src/exports.js @@ -21,7 +21,6 @@ const StyleSheet = { create(sheetDefinition /* : SheetDefinition */) { return mapObj(sheetDefinition, ([key, val]) => { return [key, { - // TODO(gil): Further minify the -O_o--combined hashes _name: process.env.NODE_ENV === 'production' ? `_${hashObject(val)}` : `${key}_${hashObject(val)}`, _definition: val diff --git a/src/inject.js b/src/inject.js index 9d17595..9a79657 100644 --- a/src/inject.js +++ b/src/inject.js @@ -269,7 +269,10 @@ export const injectAndGetClassName = ( if (processedStyleDefinitions.classNameBits.length === 0) { return ""; } - const className = processedStyleDefinitions.classNameBits.join("-o_O-"); + + const className = process.env.NODE_ENV === 'production' ? + `_${hashObject(processedStyleDefinitions.classNameBits.join())}` : + processedStyleDefinitions.classNameBits.join("-o_O-"); injectStyleOnce( className, diff --git a/tests/inject_test.js b/tests/inject_test.js index 5dde94c..6e76a4b 100644 --- a/tests/inject_test.js +++ b/tests/inject_test.js @@ -4,10 +4,12 @@ import jsdom from 'jsdom'; import { StyleSheet, css } from '../src/index.js'; import { + injectAndGetClassName, injectStyleOnce, reset, startBuffering, flushToString, flushToStyleTag, - addRenderedClassNames, getRenderedClassNames + addRenderedClassNames, getRenderedClassNames, } from '../src/inject.js'; +import { defaultSelectorHandlers } from '../src/generate'; const sheet = StyleSheet.create({ red: { @@ -34,6 +36,28 @@ describe('injection', () => { global.document = undefined; }); + describe('injectAndGetClassName', () => { + it('combines class names', () => { + const className = injectAndGetClassName(false, [sheet.red, sheet.blue, sheet.green], defaultSelectorHandlers); + assert.equal(className, 'red_137u7ef-o_O-blue_1tsdo2i-o_O-green_1jzdmtb'); + }); + + describe('process.env.NODE_ENV === \'production\'', () => { + beforeEach(() => { + process.env.NODE_ENV = 'production'; + }); + + afterEach(() => { + delete process.env.NODE_ENV; + }); + + it('creates minified combined class name', () => { + const className = injectAndGetClassName(false, [sheet.red, sheet.blue, sheet.green], defaultSelectorHandlers); + assert.equal(className, '_ymyq9s'); + }); + }); + }); + describe('injectStyleOnce', () => { it('causes styles to automatically be added', done => { injectStyleOnce("x", ".x", [{ color: "red" }], false); From bce58e964ff47134741c040bd5736a6f7a0e9006 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 13 Jun 2017 15:36:20 -0700 Subject: [PATCH 2/8] rmv underscare from _name in production --- src/exports.js | 2 +- tests/index_test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/exports.js b/src/exports.js index ae40f17..13f34be 100644 --- a/src/exports.js +++ b/src/exports.js @@ -22,7 +22,7 @@ const StyleSheet = { return mapObj(sheetDefinition, ([key, val]) => { return [key, { _name: process.env.NODE_ENV === 'production' ? - `_${hashObject(val)}` : `${key}_${hashObject(val)}`, + hashObject(val) : `${key}_${hashObject(val)}`, _definition: val }]; }); diff --git a/tests/index_test.js b/tests/index_test.js index 8856903..4069a0d 100644 --- a/tests/index_test.js +++ b/tests/index_test.js @@ -252,7 +252,7 @@ describe('StyleSheet.create', () => { }, }); - assert.equal(sheet.test._name, '_j5rvvh'); + assert.equal(sheet.test._name, 'j5rvvh'); }); }) }); From fbe5ec3bb1a1e6f3841363d9122839df1adcfeec Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 13 Jun 2017 15:42:05 -0700 Subject: [PATCH 3/8] hashString --- src/inject.js | 4 ++-- src/util.js | 2 +- tests/inject_test.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/inject.js b/src/inject.js index 9a79657..3a70899 100644 --- a/src/inject.js +++ b/src/inject.js @@ -3,7 +3,7 @@ import asap from 'asap'; import OrderedElements from './ordered-elements'; import {generateCSS} from './generate'; -import {hashObject} from './util'; +import {hashObject, hashString} from './util'; /* :: import type { SheetDefinition, SheetDefinitions } from './index.js'; @@ -271,7 +271,7 @@ export const injectAndGetClassName = ( } const className = process.env.NODE_ENV === 'production' ? - `_${hashObject(processedStyleDefinitions.classNameBits.join())}` : + `_${hashString(processedStyleDefinitions.classNameBits.join())}` : processedStyleDefinitions.classNameBits.join("-o_O-"); injectStyleOnce( diff --git a/src/util.js b/src/util.js index b12d8e0..c187244 100644 --- a/src/util.js +++ b/src/util.js @@ -132,7 +132,7 @@ export const stringifyAndImportantifyValue = ( // ordering of objects. Ben Alpert says that Facebook depends on this, so we // can probably depend on this too. export const hashObject = (object /* : ObjectMap */) /* : string */ => stringHash(JSON.stringify(object)).toString(36); - +export const hashString = (string /* : string */) /* string */ => stringHash(string).toString(36); // Given a single style value string like the "b" from "a: b;", adds !important // to generate "b !important". diff --git a/tests/inject_test.js b/tests/inject_test.js index 6e76a4b..32cfba4 100644 --- a/tests/inject_test.js +++ b/tests/inject_test.js @@ -53,7 +53,7 @@ describe('injection', () => { it('creates minified combined class name', () => { const className = injectAndGetClassName(false, [sheet.red, sheet.blue, sheet.green], defaultSelectorHandlers); - assert.equal(className, '_ymyq9s'); + assert.equal(className, '_6jmw9s'); }); }); }); From dd218b7dc2daa73fd6ba490f1669703180bcd112 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 13 Jun 2017 15:45:36 -0700 Subject: [PATCH 4/8] avoid re-hashing in production --- src/inject.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/inject.js b/src/inject.js index 3a70899..64ec893 100644 --- a/src/inject.js +++ b/src/inject.js @@ -270,9 +270,14 @@ export const injectAndGetClassName = ( return ""; } - const className = process.env.NODE_ENV === 'production' ? - `_${hashString(processedStyleDefinitions.classNameBits.join())}` : - processedStyleDefinitions.classNameBits.join("-o_O-"); + let className; + if (process.env.NODE_ENV === 'production') { + className = processedStyleDefinitions.classNameBits === 1 ? + `_${processedStyleDefinitions.classNameBits[0]}` : + `_${hashString(processedStyleDefinitions.classNameBits.join())}`; + } else { + className = processedStyleDefinitions.classNameBits.join("-o_O-"); + } injectStyleOnce( className, From d6727a357d6cc6a845dd9291dfe3da4c93503ec7 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 13 Jun 2017 15:52:47 -0700 Subject: [PATCH 5/8] fix re-hash logic and add re-hash spec --- src/inject.js | 2 +- tests/inject_test.js | 28 ++++++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/inject.js b/src/inject.js index 64ec893..a122088 100644 --- a/src/inject.js +++ b/src/inject.js @@ -272,7 +272,7 @@ export const injectAndGetClassName = ( let className; if (process.env.NODE_ENV === 'production') { - className = processedStyleDefinitions.classNameBits === 1 ? + className = processedStyleDefinitions.classNameBits.length === 1 ? `_${processedStyleDefinitions.classNameBits[0]}` : `_${hashString(processedStyleDefinitions.classNameBits.join())}`; } else { diff --git a/tests/inject_test.js b/tests/inject_test.js index 32cfba4..0cf8320 100644 --- a/tests/inject_test.js +++ b/tests/inject_test.js @@ -37,23 +37,47 @@ describe('injection', () => { }); describe('injectAndGetClassName', () => { + it('uses hashed class name', () => { + const className = injectAndGetClassName(false, [sheet.red], defaultSelectorHandlers); + assert.equal(className, 'red_137u7ef'); + }); + it('combines class names', () => { const className = injectAndGetClassName(false, [sheet.red, sheet.blue, sheet.green], defaultSelectorHandlers); assert.equal(className, 'red_137u7ef-o_O-blue_1tsdo2i-o_O-green_1jzdmtb'); }); describe('process.env.NODE_ENV === \'production\'', () => { + let prodSheet; beforeEach(() => { process.env.NODE_ENV = 'production'; + prodSheet = StyleSheet.create({ + red: { + color: 'red', + }, + + blue: { + color: 'blue', + }, + + green: { + color: 'green', + }, + }); }); afterEach(() => { delete process.env.NODE_ENV; }); + it('uses hashed class name (does not re-hash)', () => { + const className = injectAndGetClassName(false, [prodSheet.red], defaultSelectorHandlers); + assert.equal(className, `_${prodSheet.red._name}`); + }); + it('creates minified combined class name', () => { - const className = injectAndGetClassName(false, [sheet.red, sheet.blue, sheet.green], defaultSelectorHandlers); - assert.equal(className, '_6jmw9s'); + const className = injectAndGetClassName(false, [prodSheet.red, prodSheet.blue, prodSheet.green], defaultSelectorHandlers); + assert.equal(className, '_11v1ezt'); }); }); }); From a5168d7a8a165d016afbef27ae9e54c47b11dd34 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 11 Jul 2017 11:53:16 -0700 Subject: [PATCH 6/8] hashObject use hashString --- src/util.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util.js b/src/util.js index c187244..587580e 100644 --- a/src/util.js +++ b/src/util.js @@ -123,6 +123,9 @@ export const stringifyAndImportantifyValue = ( prop /* : any */ ) /* : string */ => importantify(stringifyValue(key, prop)); +// Turn a string into a hash string of base-36 values (using letters and numbers) +export const hashString = (string /* : string */) /* string */ => stringHash(string).toString(36); + // Hash a javascript object using JSON.stringify. This is very fast, about 3 // microseconds on my computer for a sample object: // http://jsperf.com/test-hashfnv32a-hash/5 @@ -131,8 +134,7 @@ export const stringifyAndImportantifyValue = ( // this to produce consistent hashes browsers need to have a consistent // ordering of objects. Ben Alpert says that Facebook depends on this, so we // can probably depend on this too. -export const hashObject = (object /* : ObjectMap */) /* : string */ => stringHash(JSON.stringify(object)).toString(36); -export const hashString = (string /* : string */) /* string */ => stringHash(string).toString(36); +export const hashObject = (object /* : ObjectMap */) /* : string */ => hashString(JSON.stringify(object)); // Given a single style value string like the "b" from "a: b;", adds !important // to generate "b !important". From af553e3271aeb86b3db5002bc39276a3f913fbfc Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Tue, 11 Jul 2017 12:25:30 -0700 Subject: [PATCH 7/8] add _len to StyleSheet definition and use it in getStyleDefinitionsLengthHash to add extra hash bit in prod --- src/exports.js | 6 ++++-- src/inject.js | 9 ++++++++- tests/inject_test.js | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/exports.js b/src/exports.js index 13f34be..33d2180 100644 --- a/src/exports.js +++ b/src/exports.js @@ -1,5 +1,5 @@ /* @flow */ -import {mapObj, hashObject} from './util'; +import {mapObj, hashString} from './util'; import { injectAndGetClassName, reset, startBuffering, flushToString, @@ -20,9 +20,11 @@ export type MaybeSheetDefinition = SheetDefinition | false | null | void const StyleSheet = { create(sheetDefinition /* : SheetDefinition */) { return mapObj(sheetDefinition, ([key, val]) => { + const stringVal = JSON.stringify(val); return [key, { + _len: stringVal.length, _name: process.env.NODE_ENV === 'production' ? - hashObject(val) : `${key}_${hashObject(val)}`, + hashString(stringVal) : `${key}_${hashString(stringVal)}`, _definition: val }]; }); diff --git a/src/inject.js b/src/inject.js index a122088..fb30cd5 100644 --- a/src/inject.js +++ b/src/inject.js @@ -243,6 +243,12 @@ const processStyleDefinitions = ( } }; +// Sum up the lengths of the stringified style definitions (which was saved as _len property) +// and use modulus to return a single byte hash value. +const getStyleDefinitionsLengthHash = (styleDefinitions /* : any[] */) /* : string */ => ( + styleDefinitions.reduce((length, styleDefinition) => length + styleDefinition._len, 0) % 36 +).toString(36); + /** * Inject styles associated with the passed style definition objects, and return * an associated CSS class name. @@ -274,7 +280,8 @@ export const injectAndGetClassName = ( if (process.env.NODE_ENV === 'production') { className = processedStyleDefinitions.classNameBits.length === 1 ? `_${processedStyleDefinitions.classNameBits[0]}` : - `_${hashString(processedStyleDefinitions.classNameBits.join())}`; + `_${hashString(processedStyleDefinitions.classNameBits.join())}${ + getStyleDefinitionsLengthHash(styleDefinitions)}`; } else { className = processedStyleDefinitions.classNameBits.join("-o_O-"); } diff --git a/tests/inject_test.js b/tests/inject_test.js index 0cf8320..90299ce 100644 --- a/tests/inject_test.js +++ b/tests/inject_test.js @@ -77,7 +77,7 @@ describe('injection', () => { it('creates minified combined class name', () => { const className = injectAndGetClassName(false, [prodSheet.red, prodSheet.blue, prodSheet.green], defaultSelectorHandlers); - assert.equal(className, '_11v1ezt'); + assert.equal(className, '_11v1eztc'); }); }); }); From 263b35f98e512a8af345d02bd34d9894e0e3c655 Mon Sep 17 00:00:00 2001 From: Gil Birman Date: Mon, 17 Jul 2017 23:19:31 -0700 Subject: [PATCH 8/8] improve comment --- src/inject.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/inject.js b/src/inject.js index fb30cd5..45e3168 100644 --- a/src/inject.js +++ b/src/inject.js @@ -245,6 +245,7 @@ const processStyleDefinitions = ( // Sum up the lengths of the stringified style definitions (which was saved as _len property) // and use modulus to return a single byte hash value. +// We append this extra byte to the 32bit hash to decrease the chance of hash collisions. const getStyleDefinitionsLengthHash = (styleDefinitions /* : any[] */) /* : string */ => ( styleDefinitions.reduce((length, styleDefinition) => length + styleDefinition._len, 0) % 36 ).toString(36);