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

[Suggestion]: Other method to fix issue rather than depending on useEffect #7248

Open
onkarhanchate14 opened this issue Oct 23, 2024 · 1 comment

Comments

@onkarhanchate14
Copy link

Summary

In Synchronizing with effects section https://react.dev/learn/synchronizing-with-effects inside learn, there's a video section where we show issues in using ref and setting play/pause methods with ref and it raises error

`function VideoPlayer({ src, isPlaying }) {
const ref = useRef(null);
if (isPlaying) {
ref.current.play(); // Calling these while rendering isn't allowed.
} else {
ref.current.pause(); // Also, this crashes.
}

return

And below it we say way to solve this is by using useEffect

But rather we can use following way instead to solve the issue. I guess we should tell readers that this can also be possible way but here we'll see how we can solve with useEffect.

`function VideoPlayer({ src, isPlaying }) {
const ref = useRef(null);
if(ref.current !== null){
if (isPlaying) {
ref.current.play(); // Calling these while rendering isn't allowed.
} else {
ref.current.pause(); // Also, this crashes.
}
}

return

Page

https://react.dev/learn/synchronizing-with-effects

Details

I think we should show readers this way of solving the null Ref issue. This will help them to understand different ways and correct usecase of useEffect.

Subsection URL - https://react.dev/learn/synchronizing-with-effects

@bondz
Copy link
Contributor

bondz commented Oct 23, 2024

@onkarhanchate14 below the sandbox it says

The reason this code isn’t correct is that it tries to do something with the DOM node during rendering. In React, rendering should be a pure calculation of JSX and should not contain side effects like modifying the DOM.

Your suggestion of checking for null with if(ref.current !== null) while it technically solves the problem isn't the correct solution either. The ref would still be modifying the DOM, using useEffect here is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants