Closed ccorcos closed 6 years ago
@ccorcos there is work being done on a top-level API for managing the render and commit phases. Check out this demo and explanation by @acdlite to get an idea of where things might be headed.
Notion is pretty cool btw. Say hi to Surganov. :-)
Interesting use case. Suggests that Redux is missing this feature, too. Something like dispatch(action, callback)
. Or a didUpdate
argument to createStore
.
I think the thing the only reason you would need a global didUpdate
is if focusTodoItem
triggers many separate updates on many components, because you don't know which component to schedule the callback on. This happens to be how Redux works today; it calls setState
on every connected component. But that's mostly an implementation detail. Conceptually, a dispatch
is a single, atomic update, as if you were to call setState
on the Provider, and the rest of the changes were propagated via context. (The only reason we don't do that is because context, uh, kinda sucks as-is :D). In this model, the didUpdate
hook you're asking for would just be a setState
callback on the Provider. Redux could expose it like dispatch(action, callback)
.
The reason you want the callback to scheduled per action is that, in the future, once we move to async, different actions may be scheduled with different priorities. So callbacks may fire at different times depending on the priority.
Slight tangent: I have this idealized, possible-future version of React Redux in my head where there is no "store" and all state is stored as component state inside the Provider. Something like this:
class Provider extends React.Component {
state = { storeState: null };
dispatch = (action, callback) => {
this.setState(state => {
const nextStoreState = reducer(state.storeState, action);
return {storeState: nextStoreState};
}, callback);
});
// ...
}
This would allow React's scheduler to abort, reorder, and rebase Redux actions according to their priority, which isn't possible today.
Though of course we'd have to figure out what this means for middleware and enhancers. And maybe fix context (though that's not necessarily a blocker, as this would still work with pub sub like we do today eh maybe not, would be hard to do pub sub in an async-friendly way).
Of course, then you don't really have "React-Redux" any more, you have "Redux in React" - which is not the same thing :)
Don't you Redux-splain and React-splain at me, acemarke :)
Thanks guys!
@acdlite you touched on a few things there. The createNewTodo
example I gave is the simplest possible example, but there are many more complicated examples that require this feature. Just looking through my application, we use it in 56 different places to do stuff like scroll things into view after they render, changing the user's selection after a user manipulation, focusing elements after they're dragged and dropped onscreen, clicking on elements after they're created, etc.
If I understand you correctly, what you were getting at is that the user's focus could be a state in redux and managed by the component itself. If you're living in Redux world, I can buy into that. But there are more complicated examples like clicking DOM elements and scrolling to elements after they appear that would be challenging to handle via state. These actions are one-off events and should probably live in the redux action.
Async renderering and reprioritizing is an interesting dilemma. There could be an API like didUpdate()
which is called after the next render, and afterFlush()
which is called when the render queue is empty.
what you were getting at is that the user's focus could be a state in redux and managed by the component itself
No, that's not what I meant to suggest. The point of the callback is to fire right after the action has been flushed to the DOM, which is what the second parameter to setState
provides: setState(update, callback)
. It's kind of like componentDidUpdate
, but it only fires once that specific update has been rendered. Not on every render. This is important in a world where different updates have different priorities, because the next render might not include the update you just scheduled. By using the setState
callback, you have a guarantee that the effect of the dispatched action has already been flushed to the DOM.
A future version of Redux could potentially expose this by adding a second argument to dispatch
: dispatch(action, callback)
. Whether or not the focus state is stored in Redux isn't important.
Ahhh. I see what you mean. Right now, I'm just waiting for all the this.forceUpdate
callbacks to complete (I'm using something similar to reactive-magic
for state management). This means that the application might not be as responsive as it could but at least it works.
As far as implementing a granular callback with setState inside Redux, especially in a pure way, that sounds challenging because you don't know which components need to update until they begin to re-render...
I suppose that means the only way we can do this at a granular level is to implement our own render queues within the state management layer. But that doesn't mean we couldnt implement a global React.didUpdate that waits for all rendering to flush.
you don't know which components need to update until they begin to re-render...
When you call dispatch
, every component that needs to update in response should update in the same render pass. That isn’t always the case today, but that’s how it should work. And how we’ll need to make it work in the future so that Redux works with React’s async rendering.
A global didUpdate
is pretty unlikely because it doesn’t scale. And besides, a setState
callback on a root/Provider-type component, as described here, provides effectively the same thing.
Want to make sure this part is clear: the only reason Redux triggers many setState
calls for a single dispatch is to workaround the issue where context
changes don’t propagate through a component whose shouldComponentUpdate
returns false. That’s why each connected Redux component has its own subscriber to the store. But ideally, we’d only call setState
on the Provider, and the connected components would update via context or some context-like mechanism. Regardless of how we implement it, you never want to have a situation where a single dispatch causes components to re-render at different times. That’s how you get “tearing,” or a momentarily inconsistent UI. It’s mostly not a big deal in today’s world because setState
is synchronous (or part of a synchronous batch). So the tears aren’t noticeable. But in an async world, tearing is more of a problem, because it might be hundreds of milliseconds before an update is flushed, depending on what’s in the queue.
Y'know, that's an excellent point - I hadn't connected those dots yet.
Tagging @jimbolla and @timdorr as an FYI.
@acdlite Has any research been done into coordinating setState
s via explicit batches, especially in an async React render cycle? You would get that implicitly with priorities, where the grouping occurs along the priority values. But I think that would be particularly helpful with coordinating sections of a UI to avoid the inconsistencies of async actions.
More concretely, I'm thinking something like this:
handleChange = event => this.batchedSetState(BATCH_ID, { foo: event.target.value })
That would group up units of work and only reconcile with the DOM after the VDOM has been fully-rendered for those components.
That would be a way to ensure sync-like behavior when connect()
updates a component.
BTW, I saw some library recently that does what you describe as a future react-redux. I forget the name, but it's basically that. I'll try to find it again...
Now I'm curious - if you've seen a lib, there's a good chance I've seen it, but nothing is immediately coming to mind.
@acdlite I see your point abut tearing -- that makes sense. And we'll need some way to ensure that two components render in sync -- that's what was going on in that demo.
Pretty much every library that I've seen uses subscribers within components, otherwise it doesn't scale well. I could image libraries using custom render queues to enqueue forceUpdates and committing them all together. That way you could have multiple render queues and run animations on a separate queue that doesn't happen in sync. Cool stuff! Any ideas when this will land in production?
This is sounding more and more like an external library feature and less like a React feature the more we talk about it...
@timdorr Yeah we've considered something like that. Probably wouldn't use batch IDs; as you say, batching in React is done by priority level. But we're also moving to model where the priorities are expressed as expiration times. The lower the priority, the later the expiration time; as the current time advances, things increase in priority and eventually "expire," at which point the rest of the work is flushed synchronously.
We group like-priority updates by rounding the expiration time. E.g. all low priority updates scheduled within ~30ms will have the same expiration time. But if you happen to start scheduling a whole bunch of updates right on the edge of a 30ms bucket, some could spill over into the next one. Doesn't even have to be a large amount of updates if you're right on the edge (say, 29.9ms).
So if you think about how Redux works today, after an action is dispatched and the store updates its state, we call setState
on all the connected components. We need a way to guarantee that all those setState
s receive the same expiration time. Maybe that's with an explicit batch API, maybe not. You don't want to encourage over-batching either, because you could get starvation: every time you add something new to the batch, it takes additional work before any of the updates in the batch can flush.
Another problem we'll have to solve with Redux is the lack of a commit phase, or synchronization phase. If someone calls getState
, but the connected components haven't updated yet, the value they get from the store is not the value that is actually rendered on the screen. Tearing. So I think the getState
API is inherently flawed in an async world.
This gets back to why we should store state in the Provider's component state and use setState
updaters, as in my comment above. It allows us to leverage React to rebase actions.
An example. If you dispatch a normal priority action A
, then a high priority action B
, what we really want to do is flush twice: once with just B
applied, but not A
. Then later, with both A
and B
in that order:
const initialState = {log: []};
function reducer(state, action) {
return {log: [...state.log, action.value]};
}
dispatch({value: 'A'}); // Normal pri
dispatch({value: 'B'}); // High pri
// First React flushes only B, because it has higher priority
// State should be {log: ['B']}
// Later, React flushes both A *and* B, in that order.
// State should be {log: ['A', 'B']}
// Note that we rebased the high pri action on top of the low pri action. Thanks, React!
But this means getting rid of "stores" as we know them today, along with things like getState
. I don't think this means we're "replacing" Redux, @markerikson, because I don't think the store is the most important part of Redux. All your reducers and most middlewares can stay the same. Most patterns likely still work, just might need to change a bit. combineReducers
is unchanged. And so on. Those are the essential parts of Redux, IMO, not the store.
Another way of saying this is that React is already a really good, sophisticated system for scheduling UI state updates. Redux is a pattern on top of that, not something that replaces it.
We’re trying to funnel API proposals into our RFC repo so please feel free to create a pull request there and we’ll review it: https://github.com/reactjs/rfcs
I’ll close this one but we appreciate the discussion!
Currently, its not easy to write global logic that executes after React has re-rendered. The
componentDidUpdate
lifecycle method works great when your logic is isolated to a component, but I've found myself more and more recently wanting a globaldidUpdate
hook baked into React.A simple example where this is useful is if you want an isolated function (perhaps a keyboard shortcut) that creates an element on the screen and then focuses it.
At Notion, we've written custom logic for doing this, but it makes upgrading with React more difficult and unstable. I think this would be useful for others too, particularly those who use Redux and are building complicated UI interactions.