Adapt to unified v9 api #670
Conversation
|
@drcmda I have a git tip for you! Do this to easily rebase your PR onto its base branch. git pull --rebase origin v9 |
src/useTransition.js
Outdated
| @@ -32,7 +32,7 @@ const makeConfig = props => { | |||
|
|
|||
| export function useTransition(input, keyTransform, props) { | |||
| // Coerce props to an object | |||
| props = callProp(props) | |||
| props = useMemo(() => callProp(props), []) | |||
There was a problem hiding this comment.
Nope, still not good enough. :)
This ends up caching props when it's an object, which isn't what we want.
There was a problem hiding this comment.
for transition im not sure tbh, it's undefined atm. i never considered it updateable, items can update, but i've never seen someone mutate the config (it also doesn't have an updater for this reason).
There was a problem hiding this comment.
For all hooks, re-rendering should always queue an update (unless a deps array is given). Just for consistency.
There was a problem hiding this comment.
They're mostly one-time slots, "enter" and "leave" only happen once, the "update" slot is a valid concern though. Initially transition didn't have "update" so i never thought about it as updatable. How do we want to define it? And is this something that v9 useTransition can do already?
There was a problem hiding this comment.
The enter and leave props should still be updatable for future enters and leaves.
PS: We may want to remove the update prop now that the update function is always returned.
There was a problem hiding this comment.
Makes sense (enter/leave). The update prop is useful, it comes from d3 and has a specific meaning. It fires for all nodes that aren't either enter or leave. Like so: https://codesandbox.io/embed/26mjowzpr Makes it possible to treat the elements that have already been transitioned as generic springs again.
There was a problem hiding this comment.
It might be cool to allow custom phases instead of (or in addition to?) the "update" phase. The custom phase would be derived from each item. I haven't thought much about it yet, but it could be useful for multi-phase transitions.
| @@ -25,6 +25,8 @@ const makeConfig = props => { | |||
| } | |||
|
|
|||
| export function useTransition(input, keyTransform, props) { | |||
| // Coerce props to an object | |||
| props = useMemo(() => callProp(props), []) | |||
There was a problem hiding this comment.
Note: This still needs fixing. 👀
aleclarson
force-pushed
the
v9-api
branch
2 times, most recently
from
5616878 to
0c37faa
Compare
aleclarson
force-pushed
the
v9
branch
4 times, most recently
from
825aba8 to
2261d9b
Compare
|
Closed in favor of #808 |
arguments.length