atom / github

:octocat: Git and GitHub integration for Atom
https://github.atom.io
MIT License
1.12k stars 395 forks source link

fix warning at #2680 #2681

Open icecream17 opened 3 years ago

icecream17 commented 3 years ago

Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.

Description of the Change

componentWillRecieveProps is now deprecated, so this pr replaces it.

https://reactjs.org/docs/react-component.html#unsafe_componentwillreceiveprops says: "If you need to perform a side effect (for example, data fetching or an animation) in response to a change in props, use componentDidUpdate lifecycle instead."

componentDidUpdate is called after updating, rather than componentWillRecieveProps. If this (or some other subtle change) happens to break atom, then I'll just use UNSAFE_componentWillReceiveProps instead.

Warning text:


Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details.

In this case, it's used for side effects (1st bullet point)

And this implies there will still be a warning when using UNSAFE_componentWillReceiveProps in strict-mode, so trying componentDidUpdate first.

Screenshot or Gif

N/A

Applicable Issues

Fixes Part of Fixes #2680

icecream17 commented 3 years ago

The tests failed, but for external reasons image

Also there's this: image

smashwilson commented 3 years ago

The tests failed, but for external reasons

Yeah, our CI seems really unhealthy 😒 . It was always a bit rickety even when this was my major focus, but it looks like entropy has made it even worse since the last time I've done work here.

codecov[bot] commented 3 years ago

Codecov Report

Merging #2681 (8b1cfaf) into master (2053290) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2681   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files         237      237           
  Lines       13213    13213           
  Branches     1900     1900           
=======================================
  Hits        12349    12349           
  Misses        864      864           
Impacted Files Coverage Δ
lib/atom/commands.js 100.00% <100.00%> (ø)

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 2053290...8b1cfaf. Read the comment docs.

icecream17 commented 3 years ago

That last commit was for the tests to run