Skip to content

Commit

Permalink
Fix JSXStyle renders styles too late (#484)
Browse files Browse the repository at this point in the history
We tried master on zeit.co and it seems that when we have the side effects in `componentDidMount` styles are rendered after the content causing transitions to apply to properties that wouldn't otherwise be animated e.g. `padding` similarly to this https://jsfiddle.net/uwbm165z/ (unfortunately I wasn't able to reproduce with React yet but here is my attempt https://codesandbox.io/s/p7q6n6r35m).

In this patch we move the side effect `styleSheetRegistry.add` to `render` making sure that `shouldComponentUpdate` doesn't trigger unnecessary re-renderings. On update we then remove the old styles early enough aka on `getSnapshotBeforeUpdate`.

@bvaughn this is a "followup" of the conversation we had in #457 (comment) Does it make sense?
  • Loading branch information
giuseppeg authored and timneutkens committed Aug 31, 2018
1 parent 9f53a74 commit 5ddef29
Showing 1 changed file with 16 additions and 16 deletions.
32 changes: 16 additions & 16 deletions src/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,6 @@ import StyleSheetRegistry from './stylesheet-registry'
const styleSheetRegistry = new StyleSheetRegistry()

export default class JSXStyle extends Component {
constructor(props) {
super(props)

// SeverSideRendering only
if (typeof window === 'undefined') {
styleSheetRegistry.add(this.props)
}
}

static dynamic(info) {
return info
.map(tagInfo => {
Expand All @@ -23,23 +14,32 @@ export default class JSXStyle extends Component {
.join(' ')
}

componentDidMount() {
styleSheetRegistry.add(this.props)
}

// probably faster than PureComponent (shallowEqual)
shouldComponentUpdate(nextProps) {
return this.props.css !== nextProps.css
return (
this.props.styleId !== nextProps.styleId ||
// We do this check because `dynamic` is an array of strings or undefined.
// These are the computed values for dynamic styles.
String(this.props.dynamic) !== String(nextProps.dynamic)
)
}

componentDidUpdate(prevProps) {
styleSheetRegistry.update(prevProps, this.props)
// Remove styles in advance.
getSnapshotBeforeUpdate(prevProps) {
styleSheetRegistry.remove(prevProps)
return null
}

// Including this otherwise React complains that getSnapshotBeforeUpdate
// is used without componentDidMount.
componentDidUpdate() {}

componentWillUnmount() {
styleSheetRegistry.remove(this.props)
}

render() {
styleSheetRegistry.add(this.props)
return null
}
}
Expand Down

0 comments on commit 5ddef29

Please sign in to comment.