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

Add opts.componentDidUpdate #86

Merged

Conversation

koulmomo
Copy link
Contributor

@koulmomo koulmomo commented Apr 8, 2018

componentWillReceiveProps is soon going to be deprecated in future versions of react.

The new recommendation for making side-effects when there is a difference between current + previous props is to do so in componentDidUpdate (see: https://reactjs.org/docs/react-component.html#componentdidupdate)

When react enables async rendering in future releases of react, this will break lifecycle methods into "render" phase and "commit" phase. Methods in the "render" phase (includes componentWillReceiveProps) are meant to be side-effect free as they might be called multiple times.

componentDidMount and componentDidUpdate are called in the "commit" phase and are the recommended place for side-effects (includes network calls). see: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

twitter thread with more context: https://twitter.com/ken_wheeler/status/983022255746768897?s=20

The current change is BC since we are only adding a new feature. (see #87 )

`componentWillReceiveProps` is soon going to be deprecated in future versions of react.

The new recommendation for making side-effects when there is a difference between current + previous props is to do so in `componentDidUpdate` (see: https://reactjs.org/docs/react-component.html#componentdidupdate)

When react enables async rendering in future releases of react, this will break lifecycle methods into "render" phase and "commit" phase. Methods in the "render" phase (includes `componentWillReceiveProps`) are meant to be side-effect free as they might be called multiple times.

`componentDidMount` and `componentDidUpdate` are called in the "commit" phase and are the recommended place for side-effects (includes network calls). see: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

twitter thread with more context: https://twitter.com/ken_wheeler/status/983022255746768897?s=20

The current change is BC since we are only adding a new feature.
@koulmomo koulmomo changed the title [feature] add opts.componentDidUpdate add opts.componentDidUpdate Apr 8, 2018
@koulmomo koulmomo changed the title add opts.componentDidUpdate Add opts.componentDidUpdate Apr 8, 2018
@codecov
Copy link

codecov bot commented Apr 8, 2018

Codecov Report

Merging #86 into master will increase coverage by 1.28%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #86      +/-   ##
==========================================
+ Coverage   74.62%   75.91%   +1.28%     
==========================================
  Files           8        8              
  Lines         134      137       +3     
  Branches       31       32       +1     
==========================================
+ Hits          100      104       +4     
  Misses         22       22              
+ Partials       12       11       -1
Impacted Files Coverage Δ
src/prepared.js 83.33% <100%> (+5.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 741f905...d508e5b. Read the comment docs.

@ganemone
Copy link
Contributor

ganemone commented Apr 9, 2018

!merge

@old-fusion-bot old-fusion-bot bot merged commit 82259aa into fusionjs:master Apr 9, 2018
@koulmomo koulmomo deleted the feature/componentDidUpdate branch April 9, 2018 21:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants