search

re-rxjs / react-rxjs

React bindings for RxJS
https://react-rxjs.org
MIT License
549 stars 19 forks source link

Revert "perf(core): use useLayoutEffect" #118

Closed josepot closed 4 years ago

josepot commented 4 years ago

This reverts commit 973669553b98befc4256a3979870de59b4d58950.

I was preparing a PR to will-this-react-global-state-work-in-concurrent-mode to update the name of the library and to use the latest version, and as I was doing that I realized that the test ability to interrupt render was not passing. It turns out that's because of the theoretical perf improvement of using useLayoutEffect rather than using useEffect. So, after having thought about this quite a bit, I'm quite positive that we should revert this commit.

codecov[bot] commented 4 years ago

Codecov Report

Merging #118 into main will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #118   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        25    -1     
  Lines          363       359    -4     
  Branches        35        35           
=========================================
- Hits           363       359    -4
Impacted Files Coverage Δ
packages/core/src/Subscribe.tsx 100.00% <100.00%> (ø)
packages/core/src/internal/useObservable.ts 100.00% <100.00%> (ø)
packages/core/src/useSubscribe.ts 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 94baef6...e372e50. Read the comment docs.

josepot commented 4 years ago

I'm merging this and publishing a patch update.