Fallback for css variables doesn't seem to be replaced correctly in combination with logical properties

[benjamingries]

Hi,

first of all: this is a great plugin and I appreciate that you are spending your (free) time to help all of us.

I don't know whether it's a bug or not but I think I found a combination where the fallback for css variables isn't replaced correctly (I'm using webpack 5.74.0 with the dependencies postcss 8.4.16, postcss-loader 7.0.1 and postcss-preset-env 7.8.1).

In the past I wrote a (scss) definition like this:

.stage__container {
...
left: var(--size, $spacer);
...
}

This works as expected.

Now I am trying to use logical properties in order to use modern css and prevent duplicate code (in the scss file) for the arabic landingpage:

.stage__container {
  ...
  inset-inline-start: var(--size, $spacer);
  ...
}

But this doesn't seem to work because in the end the fallback is rendered twice - once with the var style and its fallback and once with only the fallback (please take a look at the order):

The same behavior occurs when I'm setting the fallback as a root variable instead of the custom property:

:root {
  --size: 1rem;
}

.stage__container {
  ...
  inset-inline-start: var(--size);
  ...
}

Setting the preserve option doesn't change this behavior for me.

The only way that works for me is this one:

.stage__container {
  ...
  inset-inline-start: var(--size);
  ...
}

But then there is no fallback for older browsers (we build a global website that is accessed by many older browsers following the usage stats).

I found out that the logical properties plugin seems to handle these (incorrect) replacements and that disabling it prevents the duplicate code. But then again there is no fallback for older browsers.

Can you recreate this behavior?

Best regards,
Benjamin

[benjamingries]

For better understand: This is the part of my webpack configuration that handles the postcss replacements:

webpack.config.js

postcss.config.js

[romainmenke]

@benjamingries Thank you for reporting this!
I think the issue is worse than it seems from your report :)

:root {
  --size: 1rem;
}

.stage__container {
  inset-inline-start: var(--size);
}

becomes :

:root {
  --size: 1rem;
}

[dir="ltr"] .stage__container {
  left: 1rem;
  left: var(--size);
}

[dir="ltr"] .stage__container {
  left: 1rem;
  left: 1rem;
  left: var(--size);
}

.stage__container:dir(ltr) {
  left: 1rem;
  left: var(--size);
}

[dir="rtl"] .stage__container {
  right: 1rem;
  right: var(--size);
}

[dir="rtl"] .stage__container {
  right: 1rem;
  right: 1rem;
  right: var(--size);
}

.stage__container:dir(rtl) {
  right: 1rem;
  right: var(--size);
}

[dir="ltr"] .stage__container {
  left: 1rem;
}

.stage__container:dir(ltr) {
  left: 1rem;
}

[dir="rtl"] .stage__container {
  right: 1rem;
}

.stage__container:dir(rtl) {
  right: 1rem;
}

.stage__container {
  inset-inline-start: 1rem;
  inset-inline-start: var(--size);
}

---

.stage__container {
  inset-inline-start: var(--size, 1rem);
}

becomes :

[dir="ltr"] .stage__container {
  left: 1rem;
  left: var(--size, 1rem);
}
[dir="ltr"] .stage__container {
  left: 1rem;
  left: 1rem;
  left: var(--size, 1rem);
}
.stage__container:dir(ltr) {
  left: 1rem;
  left: var(--size, 1rem);
}
[dir="rtl"] .stage__container {
  right: 1rem;
  right: var(--size, 1rem);
}
[dir="rtl"] .stage__container {
  right: 1rem;
  right: 1rem;
  right: var(--size, 1rem);
}
.stage__container:dir(rtl) {
  right: 1rem;
  right: var(--size, 1rem);
}
[dir="ltr"] .stage__container {
  left: 1rem;
}
.stage__container:dir(ltr) {
  left: 1rem;
}
[dir="rtl"] .stage__container {
  right: 1rem;
}
.stage__container:dir(rtl) {
  right: 1rem;
}
.stage__container {
  inset-inline-start: 1rem;
  inset-inline-start: var(--size, 1rem);
}

[romainmenke]

Definitely a bug, or more precisely a whole class of bugs.

• plugin creates fallback rules (.foo {})
• plugin looks at declarations (left: 20px)
• plugin uses the PostCSS 8 visitor API

The expected execution order is by declaration order.
But this is not always so

When a declaration is inserted by another plugin the plugin in question will revisit that node. This will be out of order.

[romainmenke]

@benjamingries Fixing this will require a major refactor of postcss-logical.
We have a rework planned for postcss-preset-env version 8.

I think this bug can be fixed and released as a patch (semver) where the work for planned for postcss-preset-env version 8 would contain breaking changes.

I don't want to spend the time on two major rewrites if we can avoid it.
Is it ok for you to leave this as-is for now?

[benjamingries]

@romainmenke Thanks for your fast investigations. For me it's ok to fix this bug in version 8. Do you already have a plan when version 8 should be released?

[romainmenke]

@benjamingries Thank you again for reporting this, it is crucial for us to receive feedback like this to improve the plugins.

Also thank you for understanding.
We are hoping to release version 8 in late October - mid November.
But circumstances can always change since all the work is done outside of our day jobs :)

You can follow along a bit here :
https://github.com/csstools/postcss-plugins/milestone/6

Refactoring logical was planned for the upcoming batch of updates.

[benjamingries]

That sounds good. Let me know if there's anything I can help with.

[romainmenke]

@benjamingries We have released a new version of postcss-logical that works more like a fallback and doesn't try to be a full polyfill.

https://www.npmjs.com/package/postcss-logical

This issue is fixed because the plugin will no longer generate all these extra rules.
Thank you again for reporting this.