Add compatibility for Stylis v4

[drenckpohl]

This is based on the changes from #11, just updated with master from the base repo and with additional unit tests.

Thanks @tpict for the initial implementation.

[Andarist]

It would be nice if you could rebase that on top of @tpict changes or include him as co-author of this.

Thank you very much for this - I like that you have included additional Emotion-based tests here. I won't have time to review this right away (definitely not this week) - could you ping me about this in the next week?

[drenckpohl]

Of course, I added @tpict as a co-author on the commit. I'll ping again sometime next week.

[tpict]

Thanks very much for picking this up. Funnily enough I just found a bug in my implementation that's present in this version too. The extra scope is added to the right hand side of the child combinator rather than the left, e.g.

.parent > .child {
  color: red;
}

becomes

.parent > #my-extra-scope .child {
  color: red;
}

rather than

#my-extra-scope .parent > .child {
  color: red;
}

[Andarist]

This looks like a critical problem that should be addressed before merging

[drenckpohl]

Thanks for finding this. I'll add unit tests and start looking for a fix.

[drenckpohl]

@tpict do you have an example test case that breaks this implementation? This test passes:

test('should handle child selectors', () => {
  const actual = serialize(
      compile(
          `.some-class  > .some-child-class {
        color: green;
      }`,
      ),
      middleware([extraScopePlugin('#my-scope'), stringify]),
  )

  expect(formatCss(actual)).toMatchInlineSnapshot(`
    "#my-scope .some-class > .some-child-class {
      color: green;
    }
    "
  `)
})

[tpict]

Ah right, this is specifically when using the & selector: .some-class > &

[drenckpohl]

Hmm this is also passing:

test('should handle child selectors', () => {
  const actual = serialize(
      compile(
          `.some-class {
        & > .some-child-class {
          color: green;
        }
        .some-parent-class > & {
          color: red;
        }
        .class-one > .class-two {
          color: blue;
        }
      }`,
      ),
      middleware([extraScopePlugin('#my-scope'), stringify]),
  )

  expect(formatCss(actual)).toMatchInlineSnapshot(`
    "#my-scope .some-class > .some-child-class {
      color: green;
    }
    #my-scope .some-parent-class > .some-class {
      color: red;
    }
    #my-scope .some-class .class-one > .class-two {
      color: blue;
    }
    "
  `)
})

I haven't tried adding tests in the context of emotion yet, so I can check that next.

[drenckpohl]

I added unit tests covering different child selector cases and it's still working as expected.

[tpict]

I'll try to get you a working (broken) example tomorrow–in the mean time I can tell you that the component I'm seeing the issue with looks something like

const MyComponent: React.FC = function() {
  return <span css={{
            [`.${someVariable}:focus ~ &, .${someOtherVariable}:focus > &`]: {
          outlineColor: "red",
        },
    }}
  />;
}

[drenckpohl]

Alright, I've had some time to dig into this and the problem seems to be with an interaction with the compat plugin.

The broken selector example I'm using is .class-one:focus ~ &, .class-two:focus > &. I was able to come up with a couple solutions that work for the test suite that uses Stylis directly but the selector still breaks for the tests running in the context of Emotion, with the main difference being these omnipresentPlugins.

I added logging for the current element before and after the plugin logic to see how it's being affected, and you can see how in the input changes with/without compat.

Input CSS:

    .some-class {
      .class-one:focus ~ &, .class-two:focus > & {
        outlineColor: red;
      }
    }

Before, first element:

    {
      value: '.some-class',
      root: null,
      parent: null,
      type: 'rule',
      props: [ '.some-class' ],
      children: [],
      line: 1,
      column: 14,
      length: 0,
      return: ''
    }

After, first element:

    {
      value: '.some-class',
      root: null,
      parent: null,
      type: 'rule',
      props: [ '#my-scope .some-class' ],
      children: [],
      line: 1,
      column: 14,
      length: 0,
      return: ''
    }

Before, second element w/o compat:

    {
      value: '.class-one:focus~&,.class-two:focus>&',
      root: null,
      parent: {
        value: '#my-scope .some-class',
        root: null,
        parent: null,
        type: 'rule',
        props: [ '#my-scope .some-class' ],
        children: [],
        line: 1,
        column: 14,
        length: 0,
        return: ''
      },
      type: 'rule',
      props: [ '.class-one:focus~.some-class', '.class-two:focus>.some-class' ],
      children: [
        {
          value: 'outlineColor:red;',
          root: [Circular],
          parent: [Circular],
          type: 'decl',
          props: 'outlineColor',
          children: 'red',
          line: 3,
          column: 29,
          length: 12,
          return: ''
        }
      ],
      line: 2,
      column: 53,
      length: 31,
      return: ''
    }

Before, second element w/ compat:

    {
      value: '.class-one:focus~&,.class-two:focus>&',
      root: null,
      parent: {
        value: '#my-scope .some-class',
        root: null,
        parent: null,
        type: 'rule',
        props: [ '#my-scope .some-class' ],
        children: [],
        line: 1,
        column: 14,
        length: 0,
        return: ''
      },
      type: 'rule',
      props: [ '.class-one:focus~#my-scope .some-class', '.class-two:focus>#my-scope .some-class' ],
      children: [
        {
          value: 'outlineColor:red;',
          root: [Circular],
          parent: [Circular],
          type: 'decl',
          props: 'outlineColor',
          children: 'red',
          line: 3,
          column: 29,
          length: 12,
          return: ''
        }
      ],
      line: 2,
      column: 53,
      length: 31,
      return: ''
    }

You can see that with compat the props are updated with parent value now including the extra scope, and without it the parent value stays correct throughout. It might be possible to write something to specifically work around the compat plugin, but trying to make it work both with and without it is tricky.

I think I have a solution if it ran after compat ran over all elements, but it's made a lot more complicated due to how Serialize and Middleware run each plugin sequentially for each element before moving on to the next element.

It would also be helpful to understand better what compat is accomplishing and why it's necessary in the context of Emotion. It seems to be handling replacing & with the parent value, but I was under the impression Stylis already does that. As far as I can tell there's no way to opt out of those omnipresent plugins if you're using something like @emotion/react.

[drenckpohl]

It's also worth mentioning

.class-one:focus ~ &, .class-two:focus > &

is broken, but both

.class-one ~ &, .class-two > &

and

.class-one:focus ~ &

work.

So it's specifically the combination of the psuedo-class, parent reference, and multiple selectors that's broken here.

[Andarist]

It would also be helpful to understand better what compat is accomplishing and why it's necessary in the context of Emotion. It seems to be handling replacing & with the parent value, but I was under the impression Stylis already does that. As far as I can tell there's no way to opt out of those omnipresent plugins if you're using something like @emotion/react.

Stylis v3 behavior was to interpret this:

.scope {
  :hover {
    color: hotpink;
  }
}

as

.scope:hover {
  color: hotpink;
}

but this was not "correct" (according to the semantics coined by SCSS & co) because it should be:

.scope :hover {
  color: hotpink;
}

So this got fixed in Stylis v4 but in Emotion we've wanted to avoid this being a breaking change and thus we've introduced this "compat" plugin that tries to "fixup" the new behavior to the old one.

---

Could you add a failing test case to this PR? I will try to look into this to understand better why this breaks.

[drenckpohl]

Thanks, that context is helpful. I just pushed the changes with a broken unit test for each suite. The current version of the plugin is broken in both, due to the logic that tries to work around compat. Changing

(element.props.length === 1 && element.value.charCodeAt(0) !== 58) ||

to

element.value.charCodeAt(0) !== 58 ||

Passes all tests for the non-Emotion suite.

[evantd]

I wonder if a reasonable work-around might be to add a feature to Emotion that allows turning the compat plugin off. @emotion/cache has a compat option, but that appears to be a completely separate thing.

[Andarist]

I think that this if statement here doesn't make sense, at least for the most part. Especially parts focusing on anything else than element.parent seems dubious - they are trying to battle the logic from the compat plugin, right?

If I just apply this change:

Suggested change

	if (
	!element.parent ||
	(element.props.length === 1 && element.value.charCodeAt(0) !== 58) ||
	!element.length
	) {
	element.props = element.props
	.map((prop) => scopes.map((scope) => scope + prop))
	.reduce((scopesArray, scope) => scopesArray.concat(scope), [])
	}
	element.props = element.props
	.map((prop) => scopes.map((scope) => scope + prop))
	.reduce((scopesArray, scope) => scopesArray.concat(scope), [])

then "vanilla" tests still pass and that is good. We, of course, still need to figure out how to fix the compatibility with compat plugin though

[Andarist]

q: what if we'd serialize the current rule node and set its .return property with the scope prefixed to the output? maybe we don't have to manipulate element.props at all?

[drenckpohl]

Yeah, this change makes sense to me. It matches up with @tpict's original implementation, before the changes to account for compat.

[Andarist]

hmm, the serializing advice could work out if the received callback would be pure but in the case of Emotion it contains the rulesheet plugin so we can't just call into it freely

Maybe worth opening a discussion in the Stylis problem about this? I'm trying to think through how those 2 plugins can coexist but so far I didn't get an epiphany with the solution 😬

[drenckpohl]

Sorry, commented before seeing that second message. Can you explain more how the .return prop is used? Are there any good docs for writing plugins outside of the examples in Middleware.js?

[Andarist]

Well, this is a little bit fuzzy to me but IIRC .return holds the "final" stringified value of a given node. I think though that this is a little bit over my head - it would be best to create a discussion in the Stylis repo so maybe we could get some guidance from Sultan for this.

[drenckpohl]

Alright, just started a discussion about it in Stylis: thysultan/stylis#281

[tpict]

Thanks both of you for investigating!

[jonavila]

Hi @Andarist, anything new thoughts on how this could potentially work with the compat plugin?

[evantd]

I wonder if one viable alternative might be to change Emotion to get rid of the compat plugin and instead preprocess the style object before passing it to stylis. It seems like that would just require traversing the object structure and changing any keys that start with : to start with &:.

For that matter, if Emotion generally supported style object proprocessers and we implemented &-resolution/flattening on the input object, we could follow that up by wrapping the whole thing in an extra scope before passing it to stylis. Dealing with the objects before passing them to stylis seems easier than working with the compiled form.

[jean-pasqualini]

Here is my basic implementation,

export default function createExtraScopePlugin(scope) {
  return (element) => {
    if (element.type !== 'rule') {
      return
    }

    if (element.root?.type === '@keyframes') {
      return
    }

    element.props = element.props
    .map((prop) => (prop.startsWith(':root') || prop.startsWith(scope)) ? prop : `${scope} ${prop}`)
  }
}

I adapted it for my usecase, where i needed to have two components inside a cms page so I scoped the css.
I suggest you to create your own implementation depends on your usecase, at least you'll not have to think about every case possible only yours.