pomber / didact

A DIY guide to build your own React
https://pomb.us/build-your-own-react/
6.29k stars 531 forks source link

Add useEffect hook #15

Closed gvergnaud closed 10 months ago

gvergnaud commented 4 years ago

Hi again :)

Here is an attempt at implementing useEffect. It seems to work, but it might be closer to useLayoutEffect since I run the effect right after updating the dom...

Anyway I would be happy to know what you think of this, and how you would have implemented it

danielpox commented 3 years ago

Seems like the cleanup is never run.

alejooroncoy commented 2 years ago

Hello! The error is in the cancelEffect function, because when the cleanup is added in runEffects, we are updating it to the current version of Fiber, but on the next commit it is converted to the previous version. So, what we must do in cancelEffects is to add in the condition if it has hooks in its old version (fiber.alternate.hooks), then filter the hooks of the old version (fiber.alternate.hooks) and finally execute the cleanups. I hope I've helped ✌

halalsenpai commented 2 years ago

Hey has anyone used the useState inside this hook? doesnt seem to be working for me

FrozenIce0617 commented 2 years ago

Hey has anyone used the useState inside this hook? doesnt seem to be working for me

You need to pass wipFiber to runEffects and cancelEffects instead of fiber runEffects(fiber) -> runEffects(wipFiber)

FrozenIce0617 commented 2 years ago

@gvergnaud , it's working on my side after replace simply fiber with wipFiber . thank you for your effort.

3uler commented 10 months ago

This will not work anymore if you use another useState below the useEffect hook, because you will get a mismatch between effect and state hooks in the update procedure of the useState. You will also need to tag the state hooks and filter for the state hooks during the update.