postcss-mixins compatibility problem after update

[mdmoreau]

Thanks for the recent updates! After getting a project switched over to the latest builds, I'm seeing a new problem while using mixins. It looks like any rules declared in a block after a mixin is called don't get processed. Here's a quick example - can create a demo repo if necessary.

/* input */
@define-mixin test {
  color: blue;
}

body {
  @mixin test;

  background: red;
  font-size: 1rem;

  @media (min-width: 64em) {
    background: green;
  }

  margin: 0;
}

/* output */
body {
  color: blue;
}

@media (min-width: 64em) {
  body {
      background: green;
  }
}

The media block comes through fine, and if I move the mixin call down under a rule, then those above it will be in the build as well. I don't think this is a postcss-mixins plugin problem, as that has always worked correctly with postcss-nested.

Let me know if there's any other info I can provide or if a demo repo would help. Thanks!

[mdmoreau]

One additional note that might be helpful - if I disable nesting then the output is the following:

body {
  color: blue;

  background: red;
  font-size: 1rem;

  @media (min-width: 64em) {
    background: green;
  }

  margin: 0;
}

That looks like something postcss-nesting should be able to handle, so it almost seems like the plugins aren't running in the correct order, even though I have mixins running before preset-env.

[Antonio-Laguna]

@mdmoreau thanks for your issue and I'm sorry this is impacting your project! Can you try enabling allowDeclarationsAfterNestedRules as explained here?

The spec has changed and doesn't allow this sort of nesting. We might need to reconsider to turn this back on by default

[mdmoreau]

@Antonio-Laguna ahh that solved it - thanks! Looked at the changelog after upgrading, but didn't think to re-read the docs.

[Antonio-Laguna]

@mdmoreau the changelog isn't updated yet. Let me think about this and see what we can do about this. Thanks for testing that!

@romainmenke thoughts on this? Kind of think given the ecosystem and how things are normally on PostCSS which can create things that unfurl, we might want to consider enabling this by default unless it's false. Let's discuss this.

[mdmoreau]

@Antonio-Laguna one thing that seems a little strange to me - and maybe I'm misunderstanding how things work - but if mixins is processed first and only has a rule and not a nested selector, shouldn't that still be processed without enabling the new option? In my example it seems like the margin: 0; should get removed, but the mixin declaring color: blue; should get grouped with the background and font-size rules.

[Antonio-Laguna]

@mdmoreau the thing is that PostCSS8 makes the order of execution no longer important which is interesting and opens lots of possibilities to things to be iteratively taken care of by plugins. However, it leads to interesting unexpected behaviour.

Probably nesting is just bailing after completing its thing.

[romainmenke]

I think we best tighten up ensureCorrectMixingOfNestingRulesAndDeclarations
Currently we enforce the spec very strictly leaving little room for other plugins.

[mdmoreau]

@Antonio-Laguna okay cool - appreciate the explanation. Thanks for the quick responses and all the work on the recent updates!

[Antonio-Laguna]

I think we best tighten up ensureCorrectMixingOfNestingRulesAndDeclarations Currently we enforce the spec very strictly leaving little room for other plugins.

Agree. Let's discuss a plan of action. I hate how this will impact semver wise.

[romainmenke]

I hope this can be done in a patch release.

Ideally we would ignore any unexpected css.
This is either totally new css and a future addition or it is something handled by another plugin.

When such a plugin does it's work and this results in nested css Postcss8 should re-run the visitor for the affected nodes.

[romainmenke]

Thought about it some more and I suggest we remove ensureCorrectMixingOfNestingRulesAndDeclarations and allowDeclarationsAfterNestedRules entirely.

What I was trying to solve was people having issues in newer browsers once nesting exists for a while.

Users of postcss-preset-env who split bundles for older vs newer browsers could have CSS that works when processed by this plugin but seems broken in newer browsers.

That issue however is maybe best left too linters or dedicated test suites.
Since the issues would surface in newer browsers it is also easier to debug and pinpoint than the other way around.

@Antonio-Laguna If you agree I will prepare a pull request with the needed removals.
This would also just be a patch release.

[Antonio-Laguna]

@romainmenke ok, let's do that then!