fix: move at-rules to end of nested responsive states

[agriffis]

Summary

Move at-rules to the end of sorted styles, so that nested states can override. Fixes #288

Alternative

This almost works:

-   const variants = { ...medias, ...states }
+   const variants = { ...states, ...medias }

but it's not enough, because states can contain at-rules too. The xstyled-provided states already does, and the developer could also supply their own, for instance with @supports

so instead we explicitly move at-rules to the end, similar to the way that sortStyles works.

Test plan

Added a new test to ensure that rules are emitted in order. Without the code change, it fails like this:

 FAIL  packages/system/src/style.test.ts
  ● #style › #style › works with variants

    expect(received).toEqual(expected) // deep equality

    - Expected  - 8
    + Received  + 8

      {
        "fontFamily": "title",
    -   "&:hover": {
    +   "@media (min-width: 400px)": {
    -     "fontFamily": "title2"
    -   },
    -   "@media (min-width: 400px)": {
    +     "fontFamily": "title3",
    +     "&:hover": {
    -     "fontFamily": "title3",
    -     "&:hover": {
    +       "fontFamily": "title4"
    +     }
    +   },
    +   "&:hover": {
    -       "fontFamily": "title4"
    -     }
    +     "fontFamily": "title2"
        }
      }

      169 |           2,
      170 |         ),
    > 171 |       ).toEqual(
          |         ^
      172 |         JSON.stringify(
      173 |           {
      174 |             fontFamily: 'title',

      at Object.<anonymous> (packages/system/src/style.test.ts:171:9)

[agriffis]

Instead of the custom matcher, I simplified the test to verify key order with JSON.stringify. Amended and force-pushed.

[agriffis]

@gregberge To be honest, there's hardly a perf impact. I benchmarked but it's not even measurable without heavily exaggerated inputs, because it's a single loop over a short list that modifies the object in place, and the result is cached by getCachedVariants anyway.

Still, for an alternative, see the latest commit which just warns in development mode. The trickiest part is that the developer needs to know to insert defaultTheme.states in the middle:

theme = {
  ...defaultTheme,
  states: {
    foo: '&:foo',
    bar: '&:bar',
    ...defaultTheme.states,
    xyz: '@supports(xyz)',
  },
}

This isn't too bad but would need to be documented, and the warning should probably point to the documentation, otherwise it will be a frequent stumbling block. It also assumes they aren't overriding existing states, which is trickier to get the order right.

I can see benefits both ways. The original approach is more automatic and therefore easier. The latter approach is more explicit but forces the developer to manage their own complexity. What do you think?

[agriffis]

What you are asking to the user here is nearly impossible if it uses the base theme (with spread operator).

Right, I don't think it's a good solution either. It was merely to give you an option since you expressed concern about the original approach.

We have to sort rules in xstyled. But efficiently, only once, then cache the result.

That's how my original proposal worked because of getCachedVariants. I'll just back out the temporary commit and it sounds like we're on the same page?

[agriffis]

@gregberge This should be all set now, lemme know if not.