search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.48k stars 4.18k forks source link

Refactor blocks away from useEffect where it makes sense #48225

Open tomdevisser opened 1 year ago

tomdevisser commented 1 year ago

What problem does this address?

I just watched this video on the misuses of useEffect and it had some really interesting takes on why useEffect is used for a lot of things for which it shouldn't be used. Something that becomes extra clear since React 18 where it renders twice.

What is your proposed solution?

I'm not a React expert by any means, but I was looking through the core blocks and found a lot of places where useEffect is used for purposes that can be refactored quite easily, especially often when just updating the state. I would love to discuss this with some more advanced React devs here and see if there are any objections against refactoring these before I dive into the code and do the refactors, but I would love to help with this.

Any thoughts?

kevin940726 commented 1 year ago

Thanks for the thoughtful issue! I agree that there are multiple misuses of useEffect in this project that we should improve on. I also tried to fix some of them, but they are not trivial to fix in a fully backward-compatible way. I think the solutions are different case-by-case and require experienced React devs to create those patches and review them. I don't believe it has a high priority (though I wish it has) right now, but I'm more than happy if anyone wants to chime in and make their hands dirty! I'm willing to provide reviews or guidance for such PRs ❤️ . For now, a tracking issue might help, then we can divide the efforts into different PRs to deep-dive into the problems.

FWIW, it's not only a "tech debt" or a code quality issue, it's also causing some UX quirks like the infamous "undo traps". That's why I think it should have a higher priority once we recognize the root cause and the importance of the issue. For now, the benefits of such refactors are still a bit unclear. Perhaps some initial PRs could help us kick off the process and draw some conclusions.

Mamaduka commented 2 days ago

I think we can close this issue, otherwise we'll have to keep it open forever.

Code quality improves and degrades in large projects like this, and the code is ever-changing. But we should try and keep improvements like this in mind when creating or reviewing PRs.