styletron-react v5 discussion

[rtsao]

Reducing burden of Flow generic annotations

The requirement of Flow 0.85+ to annotate generics is definitely a bit burdensome. styletron-react@4.4.0 makes these annotations much more palatable, but having to annotate generics at all is definitely an increase in friction.

I think it is worth looking at adding an alternative API that don't require these annotations (for example a StyletronComponent). Definitely some more thinking to be done here.

Deep object merging by default

It has been brought up withStyle should be deep object merge because this is the common use case. I am inclined to agree with this, as specific transformations (such as replacements) should just use the more generic withTransform. One option is making this change (as a breaking change), but the other option is just creating new names (as part of #283) and deprecating the old ones so migration can be more gradual. Writing codemods for this should be fairly trivial in any case.

Allowing mix of arbitrary HOCs in-between Styletron HOCs

This would allow for more flexibility of composition (i.e. allowing for connect, withRouter, etc. between withStyle HOCs)

Hooks

One of the benefits of the styled-components pattern is the lack of footguns related to manual composition of atomic class strings.

React hooks has some pretty significant footguns, but relies on lint rules and convention to avoid them. We could take a similar approach with a hooks-based Styletron API by adding a lint rule to guard against manual class name composition.

[tajo]

Hooks

How about this?

const Test = () => {
  const { css } = useStyletron();
  return (
    <div className={css({ color: red })}>Red</div>
  )
}

I agree that it adds some footguns but it also removes some hurdles:

• defining and naming styled components upfront
• $prop filtering
• ref forwarding
• displayName issues
• impossible to combine Styletron's styles + 3rd party styles
• it's more readable, less typing?

Themes

It seems really useful and convenient to support themes out of the box. Also, it could be integrated into the hook:

const Test = () => {
  const { css, theme } = useStyletron();
  return (
    <div className={css({ color: theme.colors.red })}>Red</div>
  )
}

Otherwise we would force users to use a second hook just to get the theme or creating a custom useStyletron hook.

Current selector

styled('div', {
  color: 'red',
  '&.nested': { color: 'blue' }
})

would output

.a {
  color: red
}

.b.nested {
  color: blue
}

and

<div class="a b" />

This is very useful for 3rd party animation libraries like react-transition-group that add additional classNames. Afaik, there is no workaround for that in Styletron?

Deep object merging by default

Makes sense. 👍

TypeScript

It would be nice to add typedefs for v4/v5 (DefinitelyTyped/DefinitelyTyped)

// cc @nadiia @chasestarr @gergelyke

Also, I would be happy to work on those features.

[rtsao]

Hooks and compositon

Ideally a lint rule could guard against compositions such as:

const RiskyComponent = () => {
  const {css} = useStyletron();
  const dangerousComposition = `${css({color: "red"})} ${css({color: "blue"})}`;
  return (
    <div className={dangerousComposition}>Risky</div>
  )
}

Assuming that lint rule is in place, I think the only drawbacks to a hook-based API are related to composability/composition:

A nice property of the styled-components pattern is it is guaranteed that all styled components correspond to a single element, and they all can be composed/customized using the same API.

In contrast, hooks are much more flexible, which is generally nice, but by default, the styling of these components will not be composable. Instead, customizability and composability must be manually implemented.

// Composition must be manually implemented
const NonComposable = () => {
  const {css} = useStyletron();
  return <div className={css({color: "red"})}>Foo</div>;
}

const JankilyComposable = (props) => {
  const {css} = useStyletron();
  return <div className={css({color: "red", ...props.style})}>Foo</div>;
}

// No longer single element per component.
const CompoundStyled = props => {
  const { css } = useStyletron();
  return (
    <section className={css({ color: "red", ...props.containerStyle })}>
      <h1 className={css({ color: "red", ...props.titleStyle })}>
        props.title
      </h1>
      {props.children}
    </section>
  );
};

These two properties yield the following drawbacks in my opinion:

1. Styling of each component may or may not be customizable, up to implementor
2. Increased cognitive overhead of having many different ways of exposing customizability/composition

That said, I think that generally component-level composability/customizability is mostly only crucial when consuming third-party node_modules. For regular application code, I think the increased flexibility of hooks will probably be the right trade-off. And since it mostly matters for third-party library code, it is probably okay to just shift the composability/customizability burden to the maintainers of those libraries.

Theming

Aside from removing the extra line for useContext, are there any other advantages for including theming into the Styletron hook?

Current selector

I think this has been brought up before, and I'm still unsure of whether this is worth the complexity. Are there any use cases besides react-transition-group? In my opinion, animations in React are much better handled by JS/React-focused libraries such as https://www.react-spring.io/docs/hooks/basics which fully avoids the awkwardness of any selector-based styling

[tajo]

Aside from removing the extra line for useContext, are there any other advantages for including theming into the Styletron hook?

You also have to import the Context in useContext(Context), so extra two lines. And what's even worse, the path of Context is most likely relative so it can be different in every file.

It's not the biggest deal but it adds some extra friction and that can result in using some bad patterns instead like not using constants or importing values directly and not using the context.

All other CSS in JS libs (styled-components, emotion, fela...) provide <ThemeProvider>.

[rtsao]

Ah, yeah the relative context import paths would be pretty annoying to deal with.

[tajo]

EDIT:
Implemented and merged: #296

Breakpoints

Afaik, Styletron doesn't do anything to order media queries correctly. If you don't set queries disjointly (which is arguably more work) it will sooner or later break your application in a non-obvious way. I think most of developers don't know about this and it can be a big headache to debug. Possible improvements/solutions:

Warning

Add some dev mode logic that would analyze all media queries and throw a warning if there are overlaps

Set the order manually

import { Client as Styletron } from "styletron-engine-atomic";
const engine = new Styletron({
 mediaQueryOrder: [
    '(min-width: 320px)',
    '(min-width: 480px)',
    '(min-width: 720px)',
    '(min-width: 1024px)'
  ]
});

Let Styletron to sort them (opt-in)

import { Client as Styletron } from "styletron-engine-atomic";
const engine = new Styletron({
 mediaQuerySortMinWidth: true
});

User could signal that all media queries use min-width only so Styletron could safely sort them or throw a warning if max-width is detected. This could be good for most cases since "mobile first" is a usual way to go.

Wdyt @rtsao?

[rtsao]

After some poking around I noticed this package, which seems relatively lightweight (1.6kB):

https://github.com/dutchenkoOleg/sort-css-media-queries
https://bundlephobia.com/result?p=sort-css-media-queries@1.4.3

I wonder if just fully sorting media queries would be worth the cost. If we remove the code for desktop-first sorting, it should be even smaller.

Alternatively, maybe we leave this up to the user to provide the sort function if they wish:

const engine = new Styletron({
 mediaQuerySortFn: (a, b) => {
    // arbitrary
  }
});

import sort from "sort-css-media-queries";

const engine = new Styletron({
  mediaQuerySortFn: sort
});

[tajo]

I wonder if just fully sorting media queries would be worth the cost.

Do you think we could even enable it by default and just ship it as a minor version? Is there a reason why you wouldn't want to keep media queries sorted?

If we remove the code for desktop-first sorting, it should be even smaller.

If we do it this way, it probably needs to be an opt-in thing since it wouldn't cover all cases and it could be even more confusing.

To me there is some serious value to make things working out of the box for majority of people and keep necessary configuration as small as possible.

[rtsao]

There's implicit sorting based on render order, if we add sorting then it could in theory break this. However, one could argue order was never part of the public API. But to be safe it might be best to bump a major version.

If we do it this way, it probably needs to be an opt-in thing since it wouldn't cover all cases and it could be even more confusing.

Is mobile-first sorting not sufficient? It seems to me it'd be ok to be opinionated and just support one method.

[tajo]

Many things were implemented:

• media query sorting
• the useStyletron hook
• deep merging by default

Other were discussed, not prioritized, dropped:

• StyletronComponent / splitting static and dynamic styled
• Theming
• Current selector

Outstanding tasks:

• Allowing mix of arbitrary HOCs in-between Styletron HOCs, not really sure how this could be done (you would have to pass .__STYLETRON__ through the context?), maybe not so imported now
• Typescript def, nice to have, not critical

Closing this issue for now. We can open new issues for specific tasks.