From 789659c16b6d0cbfbeefec7fac1febc80304c340 Mon Sep 17 00:00:00 2001 From: Joe Lencioni Date: Sun, 5 Mar 2017 08:18:01 -0800 Subject: [PATCH] Avoid deoptimization in generateCSS() I was profiling the css() function and Chrome raised a flag on this function: > Not optimized: Bad value context for arguments value More info on this warning: https://github.com/GoogleChrome/devtools-docs/issues/53#issuecomment-51941358 Looking at the warning and the compiled version of this code, it seems to do some things with `arguments` when using the default values here, which is causing this deoptimization. ```js var selectorHandlers = arguments.length <= 2 || arguments[2] === undefined ? [] : arguments[2]; var stringHandlers = arguments.length <= 3 || arguments[3] === undefined ? {} : arguments[3]; var useImportant = arguments.length <= 4 || arguments[4] === undefined ? true : arguments[4]; ``` By removing the default values for the arguments, the deoptimization disappears. I thought about adding logic that would provide values for these arguments if they aren't defined, but since the only thing that relies on that is tests I decided to just update the tests to always pass all of the arguments. In my benchmark, this does not seem to make much of a difference but it still seems like a good idea to avoid things that the browser tells us is deoptimized. --- src/generate.js | 6 +++--- tests/generate_test.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/generate.js b/src/generate.js index 7fb7dd8..64366ec 100644 --- a/src/generate.js +++ b/src/generate.js @@ -139,9 +139,9 @@ export const defaultSelectorHandlers = [ export const generateCSS = ( selector /* : string */, styleTypes /* : SheetDefinition[] */, - selectorHandlers /* : SelectorHandler[] */ = [], - stringHandlers /* : StringHandlers */ = {}, - useImportant /* : boolean */ = true + selectorHandlers /* : SelectorHandler[] */, + stringHandlers /* : StringHandlers */, + useImportant /* : boolean */ ) /* : string */ => { const merged = styleTypes.reduce(recursiveMerge); diff --git a/tests/generate_test.js b/tests/generate_test.js index 0ed2030..1127f6d 100644 --- a/tests/generate_test.js +++ b/tests/generate_test.js @@ -81,8 +81,8 @@ describe('generateCSSRuleset', () => { }); }); describe('generateCSS', () => { - const assertCSS = (className, styleTypes, expected, selectorHandlers, - stringHandlers, useImportant) => { + const assertCSS = (className, styleTypes, expected, selectorHandlers = [], + stringHandlers = {}, useImportant = true) => { const actual = generateCSS(className, styleTypes, selectorHandlers, stringHandlers, useImportant); assert.equal(actual, expected.split('\n').map(x => x.trim()).join(''));