Introducing functional components

[ZainlessBrombie]

Details in proposal #1026
I think the two main questions are whether the use_effect hook should always run if no arguments are supplied or always, and if FunctionProvider and FunctionComponent are appropriate names

[ZainlessBrombie]

Huh build failed. Worked on my machine 🤔

[ZainlessBrombie]

Oh right wasm-pack is missing from the build server, but I'm using it to run the tests

[ZainlessBrombie]

Another question comes to mind: Should the firing of use_effect cause the component to rerender? It currently does but in retrospect i don't think it should. Instead the setstate method called inside use_effect is what should be causing a rerender.
Future optimizations include rerendering the component only once for multiple calls to setstate. I think a renderpass counter may do the trick for that. Alternatively a next_tick worker queue may be used.

[jstarry]

This is awesome! Thanks for taking on this proposal, I cannot wait to use it in my project :D

I'm really excited about adding context hooks in the future as well, it will ease a lot of state management headaches.

I have a concern over the restrictiveness of hook ordering.. it's a runtime check unfortunately. So if anyone wraps a hook in a conditional they will get panics. But maybe React has this problem too? I'm curious your thoughts on it.

In regards to use_effect, do you think it should be called after the component is mounted? Maybe promise wrapping it would do the trick.

Thanks again!

[ZainlessBrombie]

@jstarry Alright use_effect is now refactored to run on next tick 👍
Edit: Also, this reduces rerender count for sequential changes :)
Do you think functional yew is a good name or should we name it something with "hooks"?

[jstarry]

Hey, I really think this PR should be split. Use effect still needs work and will likely need some core framework changes. Can you remove all use effect code so that we have the foundation in place earlier and then can iterate on use effect implementation?

[ZainlessBrombie]

If you insist I can remove it of course, but I honestly think use_effect is too essential to skip it :/
By framework changes, do you mean the multiple render on multiple messages thing?

[ZainlessBrombie]

Also I've removed next_tick and replaced it by making message a Box<FnOnce() -> bool>. This does indeed now rely on the framework to batch renders.

[jstarry]

If you insist I can remove it of course, but I honestly think use_effect is too essential to skip it :/

Not remove, just move use_effect code to a new PR so that the changes are more clear.

[jstarry]

I like to give detailed reviews on smaller PRs. This change is already pretty big and hard to keep up with what has changed since the last time I viewed

[ZainlessBrombie]

Ah I see, sorry
I'll have to change the tests since they rely on use_effect after that I can remove use_effect :)

[ZainlessBrombie]

Alright. use_effect is out 👍

[jstarry]

Great, thanks for splitting out the use_effect code temporarily! It's fine to keep the commented out tests. As soon as you address my last few comments, we can merge this in and work on getting use_effect in!

[jstarry]

Also I've removed next_tick and replaced it by making message a Box<FnOnce() -> bool>. This does indeed now rely on the framework to batch renders.

Nice solution! And thanks for writing up #1034!

[ZainlessBrombie]

Yw :)
Alright I addressed your comments and actually also removed the commented out use_effect tests, I'll reintroduce them in the next PR.

[jstarry]

Thanks for the cleanup!

[ZainlessBrombie]

You're welcome, I appreciate your dedication to the PR and having everything orderly :)
Looking forward to use_effect