update legacy react lifecycle methods to indicate unsafe status

[tklever]

Following the react migration path laid out here https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html

Which problem is this PR solving?

• Start of work for Issue Remove UNSAFE lifecyle methods from React components #374

Short description of the changes

• Adding UNSAFE to legacy lifecycle methods for further review and update

[yurishkuro]

Excuse my lack of React knowledge, but what would this accomplish? Wouldn't this result in the methods not being called when needed?

[codecov]

Codecov Report

Merging #610 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
+ Coverage   92.69%   92.70%   +0.01%     
==========================================
  Files         227      227              
  Lines        5911     5911              
  Branches     1491     1491              
==========================================
+ Hits         5479     5480       +1     
+ Misses        391      390       -1     
  Partials       41       41              

Impacted Files	Coverage Δ
packages/jaeger-ui/src/components/App/Page.tsx	100.00% <ø> (ø)
...aeger-ui/src/components/DeepDependencies/index.tsx	100.00% <ø> (ø)
...components/DependencyGraph/DependencyForceGraph.js	85.00% <ø> (ø)
.../jaeger-ui/src/components/DependencyGraph/index.js	100.00% <ø> (ø)
...src/components/TracePage/ArchiveNotifier/index.tsx	0.00% <ø> (ø)
...ents/TracePage/TracePageHeader/SpanGraph/index.tsx	70.58% <ø> (ø)
...ents/TracePage/TraceStatistics/DetailTableData.tsx	82.60% <ø> (ø)
...onents/TracePage/TraceStatistics/MainTableData.tsx	70.96% <ø> (ø)
...eViewer/TimelineHeaderRow/TimelineViewingLayer.tsx	91.52% <ø> (+1.69%)	⬆️
...ePage/TraceTimelineViewer/VirtualizedTraceView.tsx	92.62% <ø> (ø)
... and 2 more

---

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 8279218...b76d30e. Read the comment docs.

[tklever]

Excuse my lack of React knowledge, but what would this accomplish? Wouldn't this result in the methods not being called when needed?

Actually, somewhat the opposite. In the future, the UNSAFE_* versions of these methods are the only ones that will be called.

According to to the "Gradual Migration Plan" at https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html, in react 16.3, these method names are simply aliases (although, IMO the UNSAFE_* is a nice visual indicator that there is some debt hanging around if nothing else).

In later versions of 16, the legacy methods (like componentDidMount), will throw a deprecated warning in development mode, whereas the UNSAFE_* ones will not. UNSAFE_* in this case acknowledging, "yes we're doing it the old way, and it's with intent"

In React 17+, the legacy methods (like componentDidMount) will not be called at all. Only the UNSAFE_* version will be executed in the component lifecycle.

If you're looking for a short list of benefits as I see them, this change allows the project to upgrade React through 16.X and into 17.X without any major code changes, while still giving a pretty clear indicator that there is potential refactoring to be done. This would allow component refactoring to happen at a schedule decoupled from any needed React upgrades.

[yurishkuro]

👍

[yurishkuro]

@everett980 any concerns?

[yurishkuro]

@rubenvp8510 would you be able to review?

[rubenvp8510]

This LGTM as a starting point to address the migration of those methods.

[yurishkuro]

Thanks!