Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Commit

Permalink
feat(perf): improve performance of GlamorousComponent#render (#64)
Browse files Browse the repository at this point in the history
**What**: This takes some of the ideas from @developit to make the
glamorous component render method faster

**Why**: Because that's some hot code!

**How**: copy/paste and blindly looking for other things I figured could
be faster...

I also made some changes to the rollup config so the UMD would work
again with the process.env.NODE_ENV usage.
  • Loading branch information
Kent C. Dodds authored Apr 19, 2017
1 parent 628dd82 commit 917d2a4
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 28 deletions.
11 changes: 10 additions & 1 deletion package-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,21 @@ module.exports = {
description: 'delete the dist directory and run all builds',
default: series(
rimraf('dist'),
concurrent.nps('build.es', 'build.umd.main', 'build.umd.min')
concurrent.nps(
'build.es',
'build.cjs',
'build.umd.main',
'build.umd.min'
)
),
es: {
description: 'run the build with rollup (uses rollup.config.js)',
script: 'rollup --config --environment FORMAT:es',
},
cjs: {
description: 'run rollup build with CommonJS format',
script: 'rollup --config --environment FORMAT:cjs',
},
umd: {
min: {
description: 'run the rollup build with sourcemaps',
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"rollup-plugin-commonjs": "^8.0.2",
"rollup-plugin-json": "^2.1.1",
"rollup-plugin-node-resolve": "^3.0.0",
"rollup-plugin-replace": "^1.1.1",
"rollup-plugin-uglify": "^1.0.1",
"semantic-release": "^6.3.6",
"tslint": "^5.1.0",
Expand Down
43 changes: 36 additions & 7 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,30 @@ import commonjs from 'rollup-plugin-commonjs'
import nodeResolve from 'rollup-plugin-node-resolve'
import json from 'rollup-plugin-json'
import uglify from 'rollup-plugin-uglify'
import replace from 'rollup-plugin-replace'

const minify = process.env.MINIFY
const format = process.env.FORMAT
const esm = format === 'es'
const umd = format === 'umd'
const cjs = format === 'cjs'

let targets

if (minify) {
targets = [{dest: 'dist/glamorous.umd.min.js', format: 'umd'}]
} else if (esm) {
if (esm) {
targets = [{dest: 'dist/glamorous.es.js', format: 'es'}]
} else if (umd) {
if (minify) {
targets = [{dest: 'dist/glamorous.umd.min.js', format: 'umd'}]
} else {
targets = [{dest: 'dist/glamorous.umd.js', format: 'umd'}]
}
} else if (cjs) {
targets = [{dest: 'dist/glamorous.cjs.js', format: 'cjs'}]
} else if (format) {
throw new Error(`invalid format specified: "${format}".`)
} else {
targets = [
{dest: 'dist/glamorous.umd.js', format: 'umd'},
{dest: 'dist/glamorous.cjs.js', format: 'cjs'},
]
throw new Error('no format specified. --environment FORMAT:xxx')
}

export default {
Expand All @@ -34,6 +42,13 @@ export default {
'prop-types': 'PropTypes',
},
plugins: [
umd ?
replace({
'process.env.NODE_ENV': JSON.stringify(
minify ? 'production' : 'development'
),
}) :
null,
nodeResolve({jsnext: true, main: true}),
commonjs({include: 'node_modules/**'}),
json(),
Expand All @@ -46,3 +61,17 @@ export default {
minify ? uglify() : null,
].filter(Boolean),
}

// this is not transpiled
/*
eslint
max-len: 0,
comma-dangle: [
2,
{
arrays: 'always-multiline',
objects: 'always-multiline',
functions: 'never'
}
]
*/
35 changes: 35 additions & 0 deletions src/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,41 @@ test('renders a component with theme properties', () => {
).toMatchSnapshotWithGlamor()
})

test('in development mode the theme is frozen and cannot be changed', () => {
expect.assertions(1)
const Comp = glamorous.div(
{
color: 'red',
},
(props, theme) => {
expect(() => {
theme.foo = 'bar'
}).toThrow()
return {}
},
)
render(<Comp theme={{foo: 'baz'}} />)
})

test('in production mode the theme is not frozen and can be changed', () => {
const env = process.env.NODE_ENV
process.env.NODE_ENV = 'production'
expect.assertions(1)
const Comp = glamorous.div(
{
color: 'red',
},
(props, theme) => {
expect(() => {
theme.foo = 'bar'
}).not.toThrow()
return {}
},
)
render(<Comp theme={{foo: 'baz'}} />)
process.env.NODE_ENV = env
})

test('passes an updated theme when theme prop changes', () => {
const Comp = glamorous.div(
{
Expand Down
48 changes: 28 additions & 20 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,30 +72,37 @@ function glamorous(comp, {rootEl, displayName, forwardProps = []} = {}) {
}

render() {
const {className, innerRef, ...rest} = this.props
const {toForward, cssOverrides} = splitProps(rest, GlamorousComponent)
// in this function, we're willing to sacrafice a little on
// readability to get better performance because it actually
// matters.
const props = this.props
const {toForward, cssOverrides} = splitProps(props, GlamorousComponent)
// freeze the theme object in dev mode
const theme = process.env.NODE_ENV === 'production' ?
this.state.theme :
Object.freeze(this.state.theme)
// create className to apply
const mappedArgs = GlamorousComponent.styles.map(glamorRules => {
if (typeof glamorRules === 'function') {
return glamorRules(this.props, {...this.state.theme})
const mappedArgs = GlamorousComponent.styles.slice()
for (let i = mappedArgs.length; i--;) {
if (typeof mappedArgs[i] === 'function') {
mappedArgs[i] = mappedArgs[i](props, theme)
}
return glamorRules
})
}
const {
glamorStyles: parentGlamorStyles,
glamorlessClassName,
} = extractGlamorStyles(className)
} = extractGlamorStyles(props.className)
const glamorClassName = css(
...mappedArgs,
...parentGlamorStyles,
cssOverrides,
).toString()
const fullClassName = joinClasses(glamorlessClassName, glamorClassName)
const fullClassName = `${glamorlessClassName} ${glamorClassName}`.trim()

return React.createElement(GlamorousComponent.comp, {
className: fullClassName,
ref: innerRef,
ref: props.innerRef,
...toForward,
className: fullClassName,
})
}
}
Expand Down Expand Up @@ -211,17 +218,22 @@ function extractGlamorStyles(className = '') {
const {style} = styleSheet.registered[id]
groups.glamorStyles.push(style)
} else {
groups.glamorlessClassName = joinClasses(
groups.glamorlessClassName,
name,
)
// eslint-disable-next-line max-len
groups.glamorlessClassName = `${groups.glamorlessClassName} ${name}`.trim()
}
return groups
}, {glamorlessClassName: '', glamorStyles: []})
}

function splitProps(
{css: cssOverrides = {}, ...rest},
{
css: cssOverrides = {},
// these are plucked off
className, // because they
innerRef, // should never
// be forwarded
...rest
},
{propsAreCssOverrides, rootEl, forwardProps},
) {
const returnValue = {toForward: {}, cssOverrides: {}}
Expand Down Expand Up @@ -251,9 +263,5 @@ function capitalize(s) {
return s.slice(0, 1).toUpperCase() + s.slice(1)
}

function joinClasses(...classes) {
return classes.filter(Boolean).join(' ')
}

export default glamorous
export {ThemeProvider, withTheme}
14 changes: 14 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4094,6 +4094,12 @@ lru-cache@^4.0.1:
pseudomap "^1.0.1"
yallist "^2.0.0"

magic-string@^0.15.2:
version "0.15.2"
resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.15.2.tgz#0681d7388741bbc3addaa65060992624c6c09e9c"
dependencies:
vlq "^0.2.1"

magic-string@^0.19.0:
version "0.19.0"
resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.19.0.tgz#198948217254e3e0b93080e01146b7c73b2a06b2"
Expand Down Expand Up @@ -5414,6 +5420,14 @@ rollup-plugin-node-resolve@^3.0.0:
is-module "^1.0.0"
resolve "^1.1.6"

rollup-plugin-replace@^1.1.1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/rollup-plugin-replace/-/rollup-plugin-replace-1.1.1.tgz#396315ded050a6ce43b9518a886a3f60efb1ea33"
dependencies:
magic-string "^0.15.2"
minimatch "^3.0.2"
rollup-pluginutils "^1.5.0"

rollup-plugin-uglify@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/rollup-plugin-uglify/-/rollup-plugin-uglify-1.0.1.tgz#11d0b0c8bcd2d07e6908f74fd16b0152390b922a"
Expand Down

0 comments on commit 917d2a4

Please sign in to comment.