From 5e4c8e520a703e8037947e2b9d0d6bcee8dfdd81 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Mon, 2 Jan 2017 23:20:28 -0800 Subject: [PATCH] [fix] StyleSheet registry key check --- README.md | 1 + docs/guides/rendering.md | 4 ++ src/apis/StyleSheet/registry.js | 61 +++++++++++++++---- .../__snapshots__/index-test.js.snap | 60 ++++++++++++------ src/modules/ReactNativePropRegistry/index.js | 4 +- 5 files changed, 95 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index e4444325..002ab219 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ Exported modules: * [`Vibration`](docs/apis/Vibration.md) + ## Why? There are many different teams at Twitter building web applications with React. diff --git a/docs/guides/rendering.md b/docs/guides/rendering.md index 6fe5c1b9..ddb1adfa 100644 --- a/docs/guides/rendering.md +++ b/docs/guides/rendering.md @@ -65,6 +65,10 @@ AppRegistry.runApplication('App', { }) ``` +Setting `process.env.__REACT_NATIVE_DEBUG_ENABLED__` to `true` will expose some +debugging logs. This can help track down when you're rendering without the +performance benefit of cached styles. + ## Server-side rendering Rendering using the `AppRegistry`: diff --git a/src/apis/StyleSheet/registry.js b/src/apis/StyleSheet/registry.js index 28cef65f..232296b3 100644 --- a/src/apis/StyleSheet/registry.js +++ b/src/apis/StyleSheet/registry.js @@ -56,7 +56,7 @@ const registerStyle = (id, flatStyle) => { } }).join(' ').trim(); - const key = `${prefix}${id}`; + const key = `${prefix}-${id}`; resolvedPropsCache[key] = { className }; return id; @@ -76,6 +76,7 @@ const resolveProps = (reactNativeStyle) => { if (injectedClassNames[singleClassName]) { return singleClassName; } else { + // 4x slower render style[prop] = value; } } @@ -110,11 +111,14 @@ const resolveProps = (reactNativeStyle) => { * Caching layer over 'resolveProps' */ const resolvePropsIfNeeded = (key, style) => { - if (!key || !resolvedPropsCache[key]) { - // slow: convert style object to props and cache - resolvedPropsCache[key] = resolveProps(style); + if (key) { + if (!resolvedPropsCache[key]) { + // slow: convert style object to props and cache + resolvedPropsCache[key] = resolveProps(style); + } + return resolvedPropsCache[key]; } - return resolvedPropsCache[key]; + return resolveProps(style); }; /** @@ -123,6 +127,18 @@ const resolvePropsIfNeeded = (key, style) => { const StyleRegistry = { initialize(classNames) { injectedClassNames = classNames; + + if (process.env.__REACT_NATIVE_DEBUG_ENABLED__) { + if (global.__REACT_NATIVE_DEBUG_ENABLED__styleRegistryTimer) { + clearInterval(global.__REACT_NATIVE_DEBUG_ENABLED__styleRegistryTimer); + } + global.__REACT_NATIVE_DEBUG_ENABLED__styleRegistryTimer = setInterval(() => { + const entryCount = Object.keys(resolvedPropsCache).length; + console.groupCollapsed('[StyleSheet] resolved props cache snapshot:', entryCount, 'entries'); + console.log(resolvedPropsCache); + console.groupEnd(); + }, 30000); + } }, reset() { @@ -155,6 +171,7 @@ const StyleRegistry = { // flatten the array // [ 1, [ 2, 3 ], { prop: value }, 4, 5 ] => [ 1, 2, 3, { prop: value }, 4, 5 ]; const flatArray = flattenArray(reactNativeStyle); + let isArrayOfNumbers = true; for (let i = 0; i < flatArray.length; i++) { if (typeof flatArray[i] !== 'number') { @@ -163,14 +180,32 @@ const StyleRegistry = { } } - if (isArrayOfNumbers) { - // cache resolved props - const key = `${prefix}${flatArray.join('-')}`; - return resolvePropsIfNeeded(key, flatArray); - } else { - // resolve - return resolveProps(flatArray); - } + // TODO: determine when/if to cache unregistered styles. This produces 2x + // faster benchmark results for unregistered styles. However, the cache + // could be filled with props that are never used again. + // + // let hasValidKey = true; + // let key = flatArray.reduce((keyParts, element) => { + // if (typeof element === 'number') { + // keyParts.push(element); + // } else { + // if (element.transform) { + // hasValidKey = false; + // } else { + // const objectAsKey = Object.keys(element).map((prop) => `${prop}:${element[prop]}`).join(';'); + // if (objectAsKey !== '') { + // keyParts.push(objectAsKey); + // } + // } + // } + // return keyParts; + // }, [ prefix ]).join('-'); + // if (!hasValidKey) { key = null; } + + // cache resolved props when all styles are registered + const key = isArrayOfNumbers ? `${prefix}${flatArray.join('-')}` : null; + + return resolvePropsIfNeeded(key, flatArray); } }; diff --git a/src/components/Text/__tests__/__snapshots__/index-test.js.snap b/src/components/Text/__tests__/__snapshots__/index-test.js.snap index 58bff19e..77f9d347 100644 --- a/src/components/Text/__tests__/__snapshots__/index-test.js.snap +++ b/src/components/Text/__tests__/__snapshots__/index-test.js.snap @@ -1,15 +1,25 @@ exports[`components/Text prop "children" 1`] = ` + className=" +rn-borderTopWidth:0px +rn-borderRightWidth:0px +rn-borderBottomWidth:0px +rn-borderLeftWidth:0px +rn-color:inherit +rn-display:inline +rn-font:inherit +rn-marginTop:0px +rn-marginRight:0px +rn-marginBottom:0px +rn-marginLeft:0px +rn-paddingTop:0px +rn-paddingRight:0px +rn-paddingBottom:0px +rn-paddingLeft:0px +rn-textDecoration:none +rn-whiteSpace:pre-wrap +rn-wordWrap:break-word" + style={Object {}}> children `; @@ -44,16 +54,26 @@ rn-wordWrap:break-word" exports[`components/Text prop "selectable" 1`] = ` + className=" +rn-borderTopWidth:0px +rn-borderRightWidth:0px +rn-borderBottomWidth:0px +rn-borderLeftWidth:0px +rn-color:inherit +rn-display:inline +rn-font:inherit +rn-marginTop:0px +rn-marginRight:0px +rn-marginBottom:0px +rn-marginLeft:0px +rn-paddingTop:0px +rn-paddingRight:0px +rn-paddingBottom:0px +rn-paddingLeft:0px +rn-textDecoration:none +rn-whiteSpace:pre-wrap +rn-wordWrap:break-word" + style={Object {}} /> `; exports[`components/Text prop "selectable" 2`] = ` diff --git a/src/modules/ReactNativePropRegistry/index.js b/src/modules/ReactNativePropRegistry/index.js index 38bb5514..9dca6690 100644 --- a/src/modules/ReactNativePropRegistry/index.js +++ b/src/modules/ReactNativePropRegistry/index.js @@ -17,11 +17,11 @@ const objects = {}; const prefix = 'r'; let uniqueID = 1; -const createKey = (id) => `${prefix}${id}`; +const createKey = (id) => `${prefix}-${id}`; class ReactNativePropRegistry { static register(object: Object): number { - let id = ++uniqueID; + let id = uniqueID++; if (process.env.NODE_ENV !== 'production') { Object.freeze(object); }