react-component / overflow

📦 Auto collapse box util component

overflow-git-master.react-component.vercel.app

MIT License

51 stars 36 forks source link

fix: useLayoutEffect only works on client side #18

Closed dimbslmh closed 2 years ago

dimbslmh commented 3 years ago

Fixes #6

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/react-component/overflow/5AoS6F5EiiKkxPGS7A5BWAowRzQb
✅ Preview: https://overflow-git-fork-dimbslmh-use-layout-effect-react-component.vercel.app

codecov[bot] commented 3 years ago

Codecov Report

Merging #18 (f08b0a4) into master (1bccbe8) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #18   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         6    +2     
  Lines          200       207    +7     
  Branches        62        67    +5     
=========================================
+ Hits           200       207    +7

Impacted Files	Coverage Δ
src/Overflow.tsx	`100.00% <100.00%> (ø)`
src/hooks/useLayoutEffect.tsx	`100.00% <100.00%> (ø)`
src/utils/commonUtil.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 1bccbe8...f08b0a4. Read the comment docs.

zombieJ commented 3 years ago

Seems it should move to rc-util since too many place need this.

dimbslmh commented 3 years ago

Seems it should move to rc-util since too many place need this.

@zombieJ yeah. there seems to be a PR already covering this case done by @yoyo837 https://github.com/react-component/util/pull/252

dimbslmh commented 3 years ago

@zombieJ please check out this search: https://github.com/search?q=org%3Areact-component+useLayoutEffect&type=code.

As you said, multiple packages are using the React.useLayoutEffect hook. There are different approaches in other packages:

react-component/select uses isClient in '../utils/commonUtil'

export const isClient =
  typeof window !== 'undefined' && window.document && window.document.documentElement;

react-component/motion, react-component/input-number are using canUseDom in 'rc-util/lib/Dom/canUseDom'

export default function canUseDom() {
  return !!(typeof window !== 'undefined' && window.document && window.document.createElement);
}

What would you like to do?

zombieJ commented 3 years ago

Since this warning is useless, pick strict one is enough for test env?

dimbslmh commented 3 years ago

If you want to be able to test useLayoutEffect in the test environment you could use this: https://www.npmjs.com/package/jest-react-hooks-shallow

disguised-uchiha commented 3 years ago

Please merge this branch if it's working as expected and resolving the warning issue. https://github.com/react-component/overflow/issues/6

kiyasov commented 2 years ago

Since this warning is useless, pick strict one is enough for test env?

Hi! When will the fix be released?

zombieJ commented 2 years ago

https://github.com/react-component/util/pull/252 merged. Can move to rc-util instead