Conversation
acdlite
changed the title
acdlite
force-pushed
the
expiration
branch
3 times, most recently
from
ee127ed to
796db96
Compare
|
Confirms that this makes the Triangle demo work as expected. Still need to write some unit tests, but this is ready for review. |
|
Added unit tests. I confirmed that all the existing tests, including the sync DOM ones, are passing when expiration is enabled. |
| getPublicInstance, | ||
|
|
||
| now(): number { | ||
| // Test renderer does not use expiration |
There was a problem hiding this comment.
@sebmarkbage I went back and forth on whether this should be an optional method. Landed on making it required because all renderers will need to account for features like expiration boundaries.
There was a problem hiding this comment.
Since scheduleDeferredCallback is required it seems reasonable that this would be too since they kind of pair together.
| return false; | ||
| } | ||
|
|
||
| // TODO: Better polyfill |
There was a problem hiding this comment.
Is this polyfill sufficient? Or am I overlooking something?
There was a problem hiding this comment.
It's fine. Maybe we should colocate it with ReactDOMFrameScheduling to have all these in one place?
It is also performance.now aware: https://github.com/facebook/react/blob/master/src/renderers/shared/ReactDOMFrameScheduling.js#L73
acdlite
force-pushed
the
expiration
branch
2 times, most recently
from
67388a1 to
14e68e3
Compare
| window.performance && | ||
| typeof window.performance.now === 'function' | ||
| ) { | ||
| now = function() { |
There was a problem hiding this comment.
Why not now = performance.now and now = Date.now?
There was a problem hiding this comment.
performance.now needs to be bound, but looks like Date.now doesn't. I'll update.
acdlite
force-pushed
the
expiration
branch
3 times, most recently
from
da21094 to
43360d9
Compare
acdlite
force-pushed
the
expiration
branch
2 times, most recently
from
0d4f363 to
36244ac
Compare
| } = ReactTypeOfWork; | ||
| var {Placement, Ref, Update} = ReactTypeOfSideEffect; | ||
| var {OffscreenPriority} = ReactPriorityLevel; | ||
| var {Never} = ReactFiberExpirationTime; |
There was a problem hiding this comment.
I like that this name change decouples this expiration time from the UI context in which it's used. What we really mean is 'this might never finish updating, and that's ok' and that may or may not be literally "offscreen" in the UI.
| TaskPriority, | ||
| HighPriority, | ||
| LowPriority, | ||
| OffscreenPriority, |
There was a problem hiding this comment.
haha, I guess it's not a rename after all. ("OffscreenPriority" to "Never" expirationTime)
|
Deploy preview ready! Built with commit 564c7e9 |
|
@gaearon I’ve rebased this locally, just haven’t pushed yet. I’ll work on getting all my PRs updated tomorrow. |
6 participants
An expiration time represents a time in the future by which an update should flush. The priority of the update is related to the difference between the current clock time and the expiration time. This has the effect of increasing the priority of updates as time progresses, to prevent starvation.