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

Remove atomified <-state, optimizations #42

Merged
merged 8 commits into from
Mar 25, 2019
Merged

Remove atomified <-state, optimizations #42

merged 8 commits into from
Mar 25, 2019

Conversation

lilactown
Copy link
Collaborator

This MR removes the Atomified wrapper returned by <-state. It implements a couple of new features:

  1. <-state returns a tuple [state set-state]
  2. set-state will prevent re-renders triggered by structurally equivalent state objects
  3. set-state can be used with multiple arguments like swap!: (set-state assoc :new-key "new-value")
  4. Adds a new <-value hook that can be used to optimize <-effect, <-memo, etc.

@DjebbZ
Copy link

DjebbZ commented Mar 21, 2019

Will take a longer look soon, but after a quick glance, one thing that bothers (if I can say so) is the = check. Are you assuming that the value is a js object or a CLJ data structure? I'm saying this because I've been bitten by a similar thing in Citrus, where we had a long vector of nested maps coming from the backend and only the last item was different, so this = was taking a lot of time and blocking the UI. Should it be better to leave the check out by default and let React do its check (I know the rules documented somewhere), and offer an option to actually do the check?

I haven't looked at the other changes yet in the PR.

@lilactown
Copy link
Collaborator Author

Perhaps you're right. We have an issue over at facebook/react#15154 where we're trying to figure out if there's some extra optimizations we can do w.r.t. state updates.

I think that in practice, the chance that a state operation returns a structurally equivalent but referentially different value is rare. So might be good to keep the hot path clear of that.

@orestis
Copy link
Member

orestis commented Mar 22, 2019

I've always thought that the user should be able to supply their own comparison function, because they're best positioned to know what kind of data structures they use on a case-by-case basis.

So I think an API could look like so:

[foo set-foo] (<-state 0) ;; fall back to whatever React is doing with Object.is
[foo set-foo] (<-state :foo keyword-identical?) ;; only going to use keywords
[foo set-foo] (<-state {:some "thing"} =) ;; small maps with big ramifications
[foo set-foo] (<-state {:some "thing"} identical?) ;; big maps, prefer to re-render

(the last case could be omitted as identical? is just ===).

@DjebbZ
Copy link

DjebbZ commented Mar 22, 2019

I would go further and provide nothing. The user can just do the custom comparison him/herself before calling set-foo (this is what we ended up doing in our case). The good thing with this solution is that this code doesn't even have to be in hx at all, easy to do outside.

@lilactown
Copy link
Collaborator Author

lilactown commented Mar 23, 2019

I think that you will want to be able to do the quality check in the state computation, so that you can accommodate state changes occurring between when you call set-foo and when the computation you dispatched actually occurs. This can't happen now, but will in Concurrent mode.

Another option for users is to use <-reducer instead, like the React team has suggested for most non-toy situations.

@lilactown lilactown merged commit 2befcf2 into master Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants