Remove didCommit from scheduler #10182
Conversation
Don't actually need to track this, now that we removed the infinite loop check to findNextUnitOfWork
| (nextPriorityLevel === TaskPriority || | ||
| nextPriorityLevel === SynchronousPriority) | ||
| nextPriorityLevel === TaskPriority || | ||
| nextPriorityLevel === SynchronousPriority |
There was a problem hiding this comment.
So, we can still infinite loop with async updates?
There was a problem hiding this comment.
My reasoning is that it's always ok to do an async update in componentDidUpdate because it's non-blocking. So we shouldn't ever throw this error for an async update. E.g. If you do LIMIT - 1 nested sync updates, and then an additional async update, that should be fine.
There was a problem hiding this comment.
...well that example is bad now that the check is in findNextUnitOfWork. Here's a better one: if you commit at async priority, then you have remaining time, so you commit again at a lower priority, that's not a nested update, it's just two separate priority levels. By adding this check, we know we're only counting updates that were scheduled in the previous commit phase.
The other way we could implement this is by counting the number of commits in which setState is called at least once. Then we don't have to do a priority check. Now that I think about it, that might be more elegant.
Don't actually need to track this, now that we moved the infinite loop check to findNextUnitOfWork.