Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove UNSAFE_componentWillMount lifecycle methods #611

Merged

Conversation

tklever
Copy link
Contributor

@tklever tklever commented Aug 11, 2020

Refactor components to no longer use UNSAFE_componentWillMount methods adhering to current React best practices.

Guides Used:
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#initializing-state
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#adding-event-listeners-or-subscriptions

Which problem is this PR solving?

Short description of the changes

  • Refactored some of the easier deprecated lifecycle methods.

@codecov
Copy link

codecov bot commented Aug 11, 2020

Codecov Report

Merging #611 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
- Coverage   92.79%   92.77%   -0.02%     
==========================================
  Files         227      227              
  Lines        5911     5911              
  Branches     1491     1491              
==========================================
- Hits         5485     5484       -1     
- Misses        385      386       +1     
  Partials       41       41              
Impacted Files Coverage Δ
...components/DependencyGraph/DependencyForceGraph.js 85.00% <ø> (ø)
.../jaeger-ui/src/components/DependencyGraph/index.js 100.00% <ø> (ø)
...ents/TracePage/TraceStatistics/DetailTableData.tsx 82.60% <100.00%> (ø)
...onents/TracePage/TraceStatistics/MainTableData.tsx 70.96% <100.00%> (ø)
...eViewer/TimelineHeaderRow/TimelineViewingLayer.tsx 88.13% <0.00%> (-1.70%) ⬇️

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 fe67712...386f47a. Read the comment docs.

@yurishkuro
Copy link
Member

@rubenvp8510 mind taking a look?

Refactor components to no longer use UNSAFE_componentWillMount methods adhering to current React best practices.

Signed-off-by: Tim Klever <[email protected]>
@tklever tklever force-pushed the refactor-UNSAFE-componentWillMount branch from d9c3e8a to 386f47a Compare August 12, 2020 21:11
@rubenvp8510
Copy link
Collaborator

LGTM

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@yurishkuro yurishkuro merged commit 122a59e into jaegertracing:master Aug 15, 2020
@tklever tklever deleted the refactor-UNSAFE-componentWillMount branch August 23, 2020 06:32
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 4, 2021
Refactor components to no longer use UNSAFE_componentWillMount methods adhering to current React best practices.

Signed-off-by: Tim Klever <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 4, 2021
Refactor components to no longer use UNSAFE_componentWillMount methods adhering to current React best practices.

Signed-off-by: Tim Klever <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>
vvvprabhakar pushed a commit to vvvprabhakar/jaeger-ui that referenced this pull request Jul 5, 2021
Refactor components to no longer use UNSAFE_componentWillMount methods adhering to current React best practices.

Signed-off-by: Tim Klever <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants