search

choojs / choo

:steam_locomotive::train: - sturdy 4kb frontend framework
https://choo.io/
MIT License
6.78k stars 595 forks source link

move ._handler off state #637

Closed yoshuawuyts closed 6 years ago

yoshuawuyts commented 6 years ago

State should be serializable; by moving ._handler off state we keep state a bit cleaner. Thanks!

tornqvist commented 6 years ago

Looks good! 👍

goto-bus-stop commented 6 years ago

this makes sense to me but I wonder what @marcbachmann thinks because there might've been a reason for doing it this way in #613

marcbachmann commented 6 years ago

So far the state variable was the only one which was route-specific and mutable, which was the reason I put it there. I'm good with that change. Tests for that would be good as we'll probably run into the same issue again.

yoshuawuyts commented 6 years ago

@marcbachmann just to have some context: what issue were you running into? Agree we should have some tests for this; want to make sure we test for the right things.

marcbachmann commented 6 years ago

what issue were you running into?

I was referencing to the statement in your issue description. (State should be serializable). I had no idea that we wanted to have that state object serializable. How do people use it and do we depend on that?

tornqvist commented 6 years ago

How do people use it and do we depend on that?

I've been experimenting with persisting state to a service worker (for offline support and shared state between tabs) but if you try to post a message with anything but plain serializable values it'll throw.

But yeah, we should probably document the desire to keep state serializable.

yoshuawuyts commented 6 years ago

But yeah, we should probably document the desire to keep state serializable.

yeah, we should probably write a test for that too. I'll do that in a separate patch.

yoshuawuyts commented 6 years ago

v6.10.3