postcss / autoprefixer

Parse CSS and add vendor prefixes to rules by Can I Use
https://twitter.com/autoprefixer
MIT License
21.58k stars 1.25k forks source link

OnceExit usage doesn't work well with other plugins for IE grid. #1442

Closed romainmenke closed 2 years ago

romainmenke commented 2 years ago

When combining autoprefixer with postcss-custom-properties I get unexpected CSS.

see : https://github.com/postcss/autoprefixer/commit/8476c1d7fb72d8977403bb75f02d41611fbaf272

Input :

:root {
  --grid-gap: 15px;
}

.test-grid {
    grid-gap: var(--grid-gap);
    grid-template-areas: 'area-a area-b';
    grid-template-columns: repeat(2, 1fr);
}

Actual Output :

.test-grid {
    grid-gap: 15px;
    grid-gap: var(--grid-gap);
        grid-template-areas: 'area-a area-b';
    -ms-grid-columns: 1fr var(--grid-gap) 1fr;
    grid-template-columns: repeat(2, 1fr);
}

Expected Output :

.test-grid {
    grid-gap: 15px;
    grid-gap: var(--grid-gap);
        grid-template-areas: 'area-a area-b';
    -ms-grid-columns: 1fr 15px 1fr;
    -ms-grid-columns: 1fr var(--grid-gap) 1fr;
    grid-template-columns: repeat(2, 1fr);
}

This worked correctly when autoprefixer used Once.

As far as I understand the issue seems to happen because autoprefixer takes the last value for grid-gap to make -ms-grid-columns work.

I don't know which properties are affected by this. Maybe the IE grid magic is the only thing that takes a value from another property in this way?


Is there any context for the change to OnceExit ?

ai commented 2 years ago

We moved to OnceExit to fix compatibility with other plugins.

Can we fix the problem by changing it to RootExit?

romainmenke commented 2 years ago

RootExit seems to have the same result as OnceExit.

Is it possible for autoprefixer to use Declaration, Rule,... ? I expect this to be a major overhaul :/

Currently I am still hoping there is an easy fix to this. It's also low prio for me personally as there are two workarounds :

If there is no easy fix we could simply document this in postcss-preset-env and maybe other relevant places.

ai commented 2 years ago

Is it possible for autoprefixer to use Declaration, Rule,... ?

Yes, this is a proper solution.

But prefixes are dead, so Autoprefixer is a legacy project. I do not have motivation for that. Even if I ask another person to do it, fixing bugs after so big refactoring will take too much time too.

don't use var() in grid stuff

Honestly, I think this is the best solution. IE is almost dead. It is better to have small rule (or write small Stylelint rule).

If there is no easy fix we could simply document this in postcss-preset-env and maybe other relevant places.

I agree. Can you find the best place and send PR (docs fixing are better done by other people rather than by maintainer, since I do not use Autoprefixer’s docs).

romainmenke commented 2 years ago

IE is almost dead.

🤞

I think maybe the best place to document this is the already lengthy disclaimer for the IE grid logic on Autoprefixer.

I will try submit a PR for this.

Thank you for your thoughts!

romainmenke commented 2 years ago

Has been added in postcss-preset-env docs : https://github.com/csstools/postcss-plugins/tree/main/plugin-packs/postcss-preset-env#autoprefixer

This seemed the best place to add some warning.