[RFC] Move keyframes inside the component-tree

[mxstbr]

Similar to injectGlobal (ref #1333) we need to move the keyframes`` helper inside the component-tree for two reasons: You should be able to use the theme in your animations and it makes appending by target possible. (ref #1324)

---

There's three main options I can think of here:

1. Deprecate and remove keyframes entirely: The keyframes helper isn't really necessary to style an app with styled-components, you could also just make global @keyframes declaration in your global styles.
2. Introduce a component-based API for keyframes: Similar to the change from Component-based API for global styling #1333 you'd have a const FadeIn = createKeyframesComponent`` or something API that returns a component which you can render <FadeIn />. (note that this wouldn't apply that animation to it's children or anything, like the <GlobalStyle /> component it would only inject the @keyframes declaration)
3. Make keyframes "lazy": Rather than returning the name of the keyframes from the keyframes`` call, we return a custom Keyframes object. Then, when folks interpolate said object, we check for instanceof and only then inject the @keyframes declaration into the DOM. We should also be able to walk the ruleset to resolve any interpolated functions etc.

Personally, 1. feels weird to me: The whole reason we introduced the keyframes helper initially was because it was weird to never have to worry about a global namespace, just to then use a global namespace again for animations. 2. also doesn't fit very well imo, I feel like that takes the component-based thing too far.

To me, 3. is the best of all worlds: No big API change (we'd probably still need to cut a major version if people rely on the name being returned), docs all stay the same and you can use e.g. the theme in your keyframes. (and makes #1324 possible)

Any concerns, other ideas or comments? First WIP PRs demonstrating what an implementation could look like would be appreciated!

[marionebl]

I lean towards variant 2, primarily to keep the APIs in symmetry:

const KeyFrame = createKeyframe``;
const Globals = createGlobalStyle``;

const StyledDiv = styled.div`
  animation-name: ${props => props.animationName};
`;

const Comp = () => (
  <StyledDiv animationName={KeyFrame.name}>
    <KeyFrame/>
    <Globals/>
  </StyledDiv>
);

One way to think about #1493 and #1491 is establishing a way to render styled-components output inside the HTML of your application. A nice benefit of variant 2 is that it enables to specifiy output targets in the DOM arbitrarily via ReactDOM.createPortal:

const KeyFrame = createKeyframe``;
const Globals = createGlobalStyle``;

const StyledDiv = styled.div`
  animation-name: ${props => props.animationName};
`;

const Comp = () => (
  <StyledDiv animationName={KeyFrame.name}>
    {ReactDOM.createPortal(<KeyFrame/>, document.head)}
    {ReactDOM.createPortal(<Globals/>, document.head)}
  </StyledDiv>
);

[mxstbr]

My main problem with version 2 is that people might expect this kind of thing to work:

// This is a perfectly fine name for what createKeyframes returns
const FadeIn = createKeyframes`
  0% { opacity: 0; }
  100% { opacity: 1; }
`

// This is not a fine usage for what createKeyframes returns, this wouldn't work at all
// even though the name and it being a component suggest this would work
<FadeIn>
  <MyComponent />
</FadeIn>

Which it won't since the returned component is really only responsible for injecting the @keyframes declaration and most likely wouldn't even render it's children. 😕

[kitten]

I really like 3 as I don't see keyframes being anything like injectGlobal; They're often "one off" creations, containing a single declaration, being local to a couple of components only.

Edit: Another thing is that it's never necessary to remove / unmount them. If we'd introduce special cases around that we'd have yet another special snowflake component.

Also, we can solve 3 in a way that scales for potential future usages. This could enable us to use its same static injection logic for styled components that don't change (are static w/o interpolations), which is what @schwers has already been working on

[mxstbr]

Gotta play devil's advocate for version 1 here for a second, what if we made the recommended usage of @keyframes something like this:

const FadeIn = styled.div`
  @keyframes fadeIn {
    0% { opacity: 0; }
    100% { opacity: 1; }
  }

  animation: fadeIn 1s;
`

The keyframes name would be scoped to the component by us here and thusly not sit in a global namespace (@keyframes sc-asdf123-fadeIn; if stylis doesn't do this yet we can make it happen), and you could still do everything you could do before without ever leaving the component tree. It also doesn't loose any developer ergonomics imo and it's slims our bundle size since it's one API less to ship.

[marionebl]

Ok, I think I do not understand 3, completely - how would I go about controlling the place the @keyframes get injected?

[kitten]

@mxstbr sounds pretty good as well, especially when paired with css mixins:

const fadeIn = css`
  @keyframes fadeIn {
    0% { opacity: 0; }
    100% { opacity: 1; }
  }
`

const FadeIn = styled.div`
  ${fadeIn}
  animation: fadeIn 1s;
`

[mxstbr]

@marionebl We wouldn't inject the @keyframes declaration when keyframes`` is called, we'd inject it when it's used instead, if that makes any sense.

We'd do so when we execute the interpolations by checking if an interpolation is instanceof Keyframes and calling Keyframes.inject(target) or something if it is a Keyframes object.

Pseudo-code:

interpolations.map((interpolation) => {
  if (isFunction(interpolation)) return interpolation(props);
  if (isKeyframes(interpolation)) { 
    interpolation.inject(this.context.sheetTarget) // or wherever we get the target from
    return interpolation.name;
  }
  /* ... */
})

[mxstbr]

@kitten the only thing I dislike there is the animation: fadeIn:

const FadeIn = styled.div`
  ${fadeIn}
  //         v this being a static string sucks, how am I supposed to know what the @keyframes declaration is called?! Am I supposed to look that up like a caveman?!?!
  animation: fadeIn 1s;
`

[marionebl]

@kitten @mxstbr Re : 1 What about other fadeIn call-sites though?

const FadeIn = styled.div`
  @keyframes fadeIn {
    0% { opacity: 0; }
    100% { opacity: 1; }
  }
  @keyframes bounce {
    0% { transform: none; }
    100% { transform: translateY(100%); }
  }
  animation: fadeIn 1s;
`

const SomethingElse = styled.div`
   animation-name: ${props => props.foo ? 'fadeIn' : 'bounce'}
`

[mxstbr]

@marionebl those names would be obfuscated similar to the classNames, the injected CSS would look something like this:

@keyframes FadeIn-asdf123-fadeIn {
  0% { opacity: 0; }
  100% { opacity: 1; }
}
@keyframes FadeIn-asdf123-bounce {
  0% { transform: none; }
  100% { transform: translateY(100%); }
}

.FadeIn-asfd123 {
  animation: FadeIn-asdf123-fadeIn 1s;
}

In your case, the other components' injected CSS would look something like this:

.SomethingElse-dfhg123 {
  animation-name: SomethingElse-asdf123-fadeIn;
}

Look ma, no name clashes!

[marionebl]

@marionebl We wouldn't inject the @Keyframes declaration when keyframes`` is called, we'd inject it when it's used instead, if that makes any sense, by checking if an interpolation is instanceof Keyframes and calling Keyframes.inject(target) or something on that custom object.

Re: 3 Ok, got it - so the @keyframes declaration lands wherever the current StyleSheetManager.target is set to. I think thats a nice solution to this as this is what most users will expect when using StyleSheetManager.target.

[mxstbr]

You have a point with your example though, which is also what I pointed out above, the keyframes names being static strings sucks. What gets injected in this case:

// No keyframes`` with that name interpolated 
const SomethingElse = styled.div`
   animation-name: fadeIn;
`

/* Do we inject this... */
.SomethingElse {
  animation-name: fadeIn; 
}
/* ...or this? */
.SomethingElse {
  animation-name: SomethingElse-asdf123-fadeIn;
}

Very confusing, because you could have a global @keyframes declaration outside of styled-components with the name fadeIn. 😕

So even with the above said, I don't think 1. is very nice due to the concern around the static string name.

[mxstbr]

By the way, this is a total side note but this is a cool API, like, in general:

const FadeIn = createAnimatedComponent`
  0% { opacity: 0; }
  100% { opacity: 1; }
`
<FadeIn>
  <MyComponent />
</FadeIn>

We can't replace keyframes with it in styled-components because it 99% surely doesn't cover all the keyframes use cases, but if anybody's reading this and looking for a cool package to make... wink wink

[marionebl]

Very confusing, because you could have a global @keyframes declaration outside of styled-components with the name fadeIn. 😕

Agreed.

Understanding 3 now and I think its the most elegant way to do this (almost nothing new to learn for users!).

[Fer0x]

@mxstbr maybe custom .toString() method can help us avoid all possible breaking changes, i.e. Keyframes.prototype.toString() will return generated animation name.

[evan-scott-zocdoc]

I personally find this RFC weird. But then again I'm against the componentization of things that don't make sense as components.

[mxstbr]

@evan-scott-zocdoc yep I agree, that's why I much prefer variant 3 to variant 1 😊

@Fer0x that's not a bad idea actually! Are there any edge cases where toString isn't called when you expect a string?