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

🚀 8.0.0 milestones (collecting ideas) #425

Closed
3 of 8 tasks
drcmda opened this issue Jan 8, 2019 · 46 comments
Closed
3 of 8 tasks

🚀 8.0.0 milestones (collecting ideas) #425

drcmda opened this issue Jan 8, 2019 · 46 comments

Comments

@drcmda
Copy link
Member

drcmda commented Jan 8, 2019

  • making all hooks available with full docs (useSprings, useSpring, useTrail, useTransition, useChain)
  • maybe making useSprings the base primitive internally for code re-use and leaner bundle size
  • decide on uniform api shapes, for instance useTransition(items, keys, props), useTrail(length, props), useSpring(length, props)
  • useKeyframes is probably purposeless since custom hooks fulfil all its core values. Hooks can already be scripted, chained, canceled and awaited. Maybe some of the functionality (arrays, async scripting) could be made generic so that all primitives get use it instead of just transitions and keyframes (useSpring({ to: [{ opacity: 1 }, { color: 'red' }, { opacity: 0 }] })). or to: async next => {...}, though that's what the set method already does.
  • while we're at it, do we need to at all when using hooks, should it be a separate arg, separate from the config (delay, config, immediate, etc)
  • slowly removing dependencies from class based primitives, making hooks the preferred api but allowing it at the same time to be as lean as possible without carrying around old bulk
  • turn animated/createAnimatedComponent.js into a function
  • deciding on a brand/icon

render props are safe of course and won't be affected by any of this.

@drcmda drcmda pinned this issue Jan 8, 2019
@phaistonian
Copy link

A super easy way to use useTransition for single-component mount/unmount reveals (don't think it's that easy now).

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

@phaistonian any ideas on how it could look like? I am thinking: https://codesandbox.io/s/1y3yyqpq7q?from-embed

@phaistonian
Copy link

@drcmda The fact that you need to deal with an array (i.e map) makes it a bit awkward since you are not dealing with a collection but with a single item.

Perhaps, an additional method useTransition as a wrapper would do it - semantically and in terms of simplicity.

Then again, I am not sure else it could be implemented - without a holding array, that is.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

That's the thing, transitions are always arrays. If you have a single item and you click it twice, you have one item fading out, and one fading in. There is a flag (unique = true) that allows it to make these transforms on the item that's currently there, but that could mean unintented behaviour.

I mean, something like this could be possible, just not sure if that doesn't dirty the waters too much for a single usecase

const transitions = useTransition(show, { opacity: 0, enter: { opacity: 1 }, leave: { opacity: 0 } })
return (
  <div class="main" onClick={() => set(state => !state)}>
    {transitions.map(
      ([show, props, key]) =>
        show && <animated.div key={key} style={props}>hello</animated.div>
    )}
  </div>
)

@phaistonian
Copy link

Well, the second example feels way more elegant and natural.

I get that offloading the lack of an array to animated may trigger issues and/or will require refactoring, however, providing a simplified and semantically appealing API always pays back - especially in the long run.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

Need to think about it a little. Maybe it would make sense to return a single object instead of an array if you supply a single item to useTransitions, that way it wouldn't change input/output semantics.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

@phaistonian what do you expect should happen here?

const [name, set] = useState("robert")
const { item, props } = useTransition(name, { opacity: 0, enter: { opacity: 1 }, leave: { opacity: 0 } })

useEffect(() => {
  setTimeout(() => set("tina"), 100)
  setTimeout(() => set("thomas"), 200)
  setTimeout(() => set("maria"), 300)
  setTimeout(() => set("philipp"), 400)
}, [])

return <animated.div style={props}>{item}</animated.div>)

With the current impl it's no problem, it'll return a list of items, the names come in, then fade out and the last remaining will be philipp while the others will safely move out. Without a list, i don't think this can be done ...

@phaistonian
Copy link

I am not sure what would happen, I am sure that it looks great - in terms of simplicity.

It all comes to how much of complexity you can hide inside animated.

Animating a single component using .map feels wrong and this feels right.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

animated.element is just to receive animated styles. The problem is that this is plain React now, it's one div, and we need 5 to safely animate. It looks good, sure, but if it really did that it would be pure, unexplainable magic. And don't get me wrong, i love simplicity. But at the same time i dislike implicit magic. If you have ideas how to find the balance in between these two i'm happy to explore ...

@phaistonian
Copy link

I guess if you want less magic and a more declarative approach, you could always switch to old good <Transition>.

Hooks are meant to be magical :)

@cusxio
Copy link

cusxio commented Jan 9, 2019

Here's a feedback,

https://github.com/react-spring/react-spring/blob/c8f5d9a792f42cdd737235ad08323c076f20ac86/src/hooks/useTransition.js#L259-L264

Currently, useTransition returns props.

However, for stateless functional components in React, the default name for props is props. This causes eslint/no-shadow rule to trigger.

So the user would need to de-structure rename, e.g.

function Box(props) {
  const { item, props: styles } = userTransition(..);

  return (..);
}

From the docs, it seems that react-spring is using the term props extensively, so I'm not sure what it should be renamed to.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

Good point, it could be anything, doesn't have to be props. The class based implementation is lucky to use two functions (items => props => view), you can name it props, but it could be anything. In useTransitions case it has to map through a set of transitions, so it can either be an object, or an array:

transitions.map(
  ([item, props, key]) => <animated.div key={key} style={props} children={item} />
)

If it's an array you can name it yourself.

@drcmda
Copy link
Member Author

drcmda commented Jan 9, 2019

@phaistonian this could be a way:

const transition = useTransition(show, { opacity: 0, enter: { opacity: 1 }, leave: { opacity: 0 } })
return (
  <div class="main" onClick={() => set(state => !state)}>
    {transition((show, props, key) =>
      show && <animated.div key={key} style={props}>hello</animated.div>
    )}
  </div>
)

This is very similar to the api i use in react-with-gesture

@phaistonian
Copy link

Yes, this looks proper, however, the rest of the hooks yield an object, not a function - if I got it right - so going for this might be a bit inconsistent.

@PascalSenn
Copy link

hm..
you may want to keep the deconstructed object.
You'll get less breaking changes in the api when you add new stuff. Deconstructed objects are always easier to extend.

Probably naming it styles would be better.

@desmap
Copy link

desmap commented Jan 10, 2019

I am coming a bit late to the party but here my feedback:

  • Re all the top ideas, they sound great to me, let's go for it and keep on the great work
  • Re to, I haven't it used it yet and I am exclusively on hooks
  • Re the useTransitions discussion, not sure if it's exactly related but what I like to do with page transitions:

I have a Navigation component which usually wraps all pages, don't have a Codesandbox now but the idea is quite simple:

// Router wraps the Navigation component
function Navigation(props) {
  <animated.div>
    {props.children} // here are pages
  </animated.div>
}

To get now page routing crossfaded I need to put useSpring and useEffect boilerplate in every page component. This is ok and manageable but it would be easier just to wrap {props.children} once in the Navigation component with some useTransition thing without giving any details like ids about the pages.

However, I think that react-spring is compared to all other libs by far the best. It has a bit steep learning curve but once you got over the first mountain it feels quite natural.

@JasCodes
Copy link

JasCodes commented Jan 15, 2019

@drcmda this is awesome, please add it :)

const transition = useTransition(show, { opacity: 0, enter: { opacity: 1 }, leave: { opacity: 0 } })
return (
  <div class="main" onClick={() => set(state => !state)}>
    {transition((show, props, key) =>
      show && <animated.div key={key} style={props}>hello</animated.div>
    )}
  </div>
)

@human909
Copy link

  • SSR rendering.
  • SSR rendering with page transitions
  • SSR rendering with page transitions with Next.js ^^

@drcmda
Copy link
Member Author

drcmda commented Jan 18, 2019

@human909 SSR will be easier because hooks give you more control. Previously render props would fire async and one frame behind (due to the deprecation of componentWillUpdate in exchange for componentDidUpdate), with hooks the view is not called, it is a given, and it will always receive data on first mount.

@JasCodes
Copy link

JasCodes commented Jan 19, 2019

@drcmda Regarding transitions sometimes one wants to wait for transition to end before starting animating in, can we create noval way to do this synchronization

@drcmda
Copy link
Member Author

drcmda commented Jan 19, 2019

if the api is easy enough why not, currently you can trail, but that will just add up a staggered small delay for enter/update and leaves.

@Jessidhia
Copy link
Contributor

Separating to from the hook config object (and from for that matter) could potentially massively simplify the type definitions.

@drcmda
Copy link
Member Author

drcmda commented Jan 21, 2019

@Jessidhia do you have any suggestions for this? im a little scared to do:

useTransition(items, itemKeys, initial, from, enter, leave, update, config)

tbh. Or is it just for useSpring?

useSpring(props, config)

But then that sort of creates two apis.

Props aren't optimal right now for sure, if we find a good model i'll gladly switch to it in 8.x

@drcmda
Copy link
Member Author

drcmda commented Jan 21, 2019

And to give you all a small update, i am almost through with a refactor of the internals of this lib to establish true determinism, which is sorely needed for chaining and orchestrating. I believe at this point it is the only lib that survives @aholachek 's https://alex.holachek.com/react-animation-comparison stress-test without a blemish. That broke a few other things i have to look into, nothing major the terrible part of the work is probably over.

Then api changes. Then making a beta. Then publish. 🤩

@Jessidhia
Copy link
Contributor

@drcmda I'm not entirely sure, but at least making it so that the to is not automatically derived from any non-blacklisted keys helps 🤔

I might be able to take a closer look at this stuff next week (release crunch at work 😅)

@Jessidhia
Copy link
Contributor

(Interesting, I hadn't looked at that react-animation-comparison thing before. It seems that Popmotion Pose is the only library to implement the shuffle items animation?)

@drcmda
Copy link
Member Author

drcmda commented Jan 22, 2019

Yes, the catch is that you basically do

<Magic>
    {items.map(Item => <item />)}
</Magic>

In react-spring some things are deliberately kept explicit, it won't clone/mutate your styles or assume positioning. If you want shuffling or a grid it's up to you, and now it can be everything, even non uniform placing: https://twitter.com/0xca0a/status/1083435616698798081 (this grid is 3 lines of code).

In the freshtiledsoil storybook i am looking at how elements behave after clicking wildly and how they transition in sync. I can destroy every attempt by clicking it dead. Including react-spring. But the new stuff will fix it: #428 which prepares it for everything async 😇

@Jessidhia
Copy link
Contributor

Jessidhia commented Jan 22, 2019

Some adventures in trying to use useSpring in real code:

  • 7.x hooks are broken in current react unless you monkey patch React.useImperativeMethods = React.useImperativeHandle 🤷‍♀️
  • useSpring's types and its need to guess types from properties makes it really hard to use in some non-trivial use cases
    • It makes autocomplete really hard to use. The CSSProperties overload might have helped for the basic use case but it's nearly impossible to find one of the config props in the list
    • Callbacks can't be inferred (cause implicit anys) because the parameter itself has to be generic, and they aren't memoized anyway so if you did give a onRest inline it'd invalidate the updateCtrl on every render
      • Normally you want function results to be as referentially stable as possible (approach useReducer's behavior as much as possible)
    • It seems useSpring({ from: { x: 0 }, to: { x: 1 }) will guess it wrong, think you're trying to interpolate an object with from and to keys, and so the types will return an object with no keys because they're on the blacklist. I even tried to add handling specifically for that when I wrote the types but that didn't work...
  • I actually had to make use of the 3rd result of useSpring to stop the animation if the user interacted with the UI mid-animation. The same can be done with the ref prop but that's not implemented in the types either...
  • The SetUpdateFn from useSpring's function-like form doesn't accept config props in the types even though it should
    • Personally, I think changing the function API to be more useReducer-like would make it easier to use. Probably not by receiving "previous state", only the "next value".
    • Something like this, where you don't have to pass a full config object every time which would basically be duplicating your initial spring function:
const [,setValue] = useSpring(value => ({ to: { opacity: value } }), 0)
setValue(1)
  • The current types of useSpring return values for native: false but native is true by default (I already commented about this with a possible solution in a PR in December or so)
  • There isn't a direct way to convert useSpring values to an imperative task. It is very likely a bad idea anyway, but for example a component that doesn't expose scrollLeft but has a scrollTo method in its ref can't be animated without adding an extra wrapper.
    • It would have been bad for performance anyway, FLIP transforms are better (scroll to target position, do a transform from the inverse of the difference in scroll, animate it to 0), but that forces me to use reset and the scrollbar behaves oddly mid-animation.
    • There also seems to be no way to synchronize the call to scrollTo with the start of the actual animation other than by racing requestAnimationFrames. onStart is just called as soon as the controller schedules it, but not when the frameLoop actually starts.

Have you made any significant changes in the 8.0 API yet, btw? If I have another chance I'd like to make cleaner types for this... when I wrote them I read a lot of the source code but didn't really experience the ergonomics.

@drcmda
Copy link
Member Author

drcmda commented Jan 22, 2019

@Jessidhia yes, if you have time you can check out the hooks-8.x branch.

cd examples
yarn
yarn start

you can run all the demos (examples/index.js) or debug single ones. this is where i plan to implement the api changes first before merging back to master. It could be that some of the issues you mentioned are already fixed in there because it's almost a complete re-do. the api is mostly still the same, though, with the exception that now you can do:

useSpring({ x: 0 }) // single values
useSpring({ xy: [0,0] }) // arrays

useSpring({ to: [{ x: 0 }, { x: 1 }] }) // executes in chain, one after the ofter

useSpring({ to: async next => {
  await next({ x: 0 })
  await fetch('www.url.com/json')........
  await next({ x: 1 })
})

No idea how a TS friendly api would look like, though, at least not without pestering the user with boilerplate.

There isn't a direct way to convert useSpring values to an imperative task. It is very likely a bad idea anyway, but for example a component that doesn't expose scrollLeft but has a scrollTo method in its ref can't be animated without adding an extra wrapper.

const AnimatedComponent = animated(ScrollToComponent)

...

const props = useSpring({ scroll: 100, from: { scroll: 0 } })

<AnimatedComponent scrollTo={props.scroll} />

or

const [value, set] = useState(0)
const props = useSpring({ scroll: 100, from: { scroll: 0 }, onFrame: set })

<ScrollToComponent scrollTo={value} />

@Jessidhia
Copy link
Contributor

That's the thing, it's not a prop, it's a method on the instance. I can do it by writing a wrapper with that interface, of course.

@drcmda
Copy link
Member Author

drcmda commented Jan 22, 2019

I see. Well i'm open to all proposals at this point. I'm guessing react-native must have the same issue, do you know per chance if they have managed to solve the typing woes around that? For certain you could prototype your ideal api here and we can discuss it.

@einarlove
Copy link
Contributor

einarlove commented Jan 31, 2019

More documentation for the spring configuration and how the different properties affect each other. Spring logic is foreign for many. What is mass etc.

I often struggle to try to tweak my springs to match a more ease-in-out feel (without using easings) and often fail.

Maybe react-spring (or a separate package) could give some tools for the developers to tweak animations live? Some inspirations could be GsapTools or animations tab in Chrome Devtools.

@drcmda
Copy link
Member Author

drcmda commented Jan 31, 2019

dan abramov made a hook for this once: https://twitter.com/dan_abramov/status/1058834904207761409 would be helpful to include something like this in the docs for sure

@einarlove
Copy link
Contributor

einarlove commented Jan 31, 2019

Would be awesome if we could use <animated.div debug> so the user does not have to add anything new to render.

@fnpen
Copy link

fnpen commented Feb 3, 2019

You forgot about SSR feature, currently incompatible(partial support, only in hooks).

@phaistonian
Copy link

An update on what has changed in API - so we can switch to using BETA?:)

ps: Since hooks are coming tomorrow, switching would make a bit more sense, I presume.

@drcmda
Copy link
Member Author

drcmda commented Feb 4, 2019

Here's a preview: http://react-spring-hooks.surge.sh

The beta is live under 8.0.0-beta.4

@phaistonian
Copy link

Wow, @drcmda. You have certainly been busy.

It's just hooks now, yes? Components are gone, true?

@drcmda
Copy link
Member Author

drcmda commented Feb 4, 2019

No class components are all there, it's just import { Spring, ... } from 'react-spring/renderprops'

@JasCodes
Copy link

JasCodes commented Feb 6, 2019

@drcmda What do you think moving project to typescript, you can gradually do it by first editing hooks calls in typescript, its types anyways have to be rewritten

@drcmda
Copy link
Member Author

drcmda commented Feb 6, 2019

not without help. i don't use it or have never used it at least. but i wouldn't be opposed to it.

@phaistonian
Copy link

@drcmda Well, hooks are officially out.

As far as I have noticed b5 is super solid (too).

Anything major other than docs to get it released?:)

@drcmda
Copy link
Member Author

drcmda commented Feb 6, 2019

it's already published (docs, too) ... i just wait and see if we can get the new website running, if not i will announce later on this day. 😇

@drcmda drcmda unpinned this issue Feb 8, 2019
@phaistonian
Copy link

@drcmda Quick q: animating 'auto' is not supported? Is there such a magic trick we could use to get "auto" height or should we deploy an extra hook to measure?

@drcmda
Copy link
Member Author

drcmda commented Feb 11, 2019

here's an example: https://twitter.com/0xca0a/status/1094683974679621633

there are two demos for auto: https://github.com/react-spring/react-spring-examples the new docs website will be published tomorrow hopefully. but you can clone this already and start it.

basically, hooks don't render, they don't know the view, so "auto" is gone. but doing it is easier than ever, and far more powerful. like the tree demo above, that's a worst-case nested auto scenario, that wouldn't have possible before.

there will be tons of hooks for this, react-measure or react-resize-aware are the ones i know.

I will add a section for this in the docs.

@phaistonian
Copy link

That's fantastic.

I came up with something similar but, for a moment, felt an anti-pattern for me.

Thank you!

@pmndrs pmndrs locked as resolved and limited conversation to collaborators Apr 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests