bdkjones / CodeKit

CodeKit 3 Issue Tracker
https://codekitapp.com
82 stars 5 forks source link

Not applying custom target browsers config to Autoprefixer #614

Closed alex-santiago closed 4 years ago

alex-santiago commented 4 years ago

Quick, short summary: It seems that Autoprefixer is not applying the custom target browser config. I ran the same file on Autoprefixer CSS online using the same target Browsers, and it added the prefixes for flex and transform.

Expected results:

/*
* Prefixed by https://autoprefixer.github.io
* PostCSS: v7.0.29,
* Autoprefixer: v9.7.6
* Browsers: >0.2%,last 2 versions,Firefox ESR,not dead,ie 9-11
*/

.some-class {
    display: -ms-flexbox;
    display: flex;
    -webkit-transform: rotate(30deg);
        -ms-transform: rotate(30deg);
            transform: rotate(30deg);
}

::-webkit-input-placeholder {
    color: gray;
}

::-moz-placeholder {
    color: gray;
}

:-ms-input-placeholder {
    color: gray;
}

::-ms-input-placeholder {
    color: gray;
}

::placeholder {
    color: gray;
}

.image {
    background-image: url(image@1x.png);
}

@media (-webkit-min-device-pixel-ratio: 2), (min-resolution: 2dppx) {
    .image {
        background-image: url(image@2x.png);
    }
}

Actual results:

.some-class {
  display: flex;
  transform: rotate(30deg); }

::-webkit-input-placeholder {
  color: gray; }

::-moz-placeholder {
  color: gray; }

:-ms-input-placeholder {
  color: gray; }

::-ms-input-placeholder {
  color: gray; }

::placeholder {
  color: gray; }

.image {
  background-image: url(image@1x.png); }

@media (-webkit-min-device-pixel-ratio: 2), (min-resolution: 2dppx) {
  .image {
    background-image: url(image@2x.png); } }
/*# sourceMappingURL=prefixes.css.map */

Exact steps to reproduce:

Using the demo on Autoprefixer's repo with an additional class that should be prefixed for ie 9. I changed the default string for target browsers to add support for IE 9 to IE 11. Target Browsers: >0.2%,last 2 versions,Firefox ESR,not dead,ie 9-11

Run the tests on https://autoprefixer.github.io/

.some-class {
    display: flex;
    transform: rotate(30deg);
}

::placeholder {
    color: gray;
}

.image {
    background-image: url(image@1x.png);
}

@media (min-resolution: 2dppx) {
    .image {
        background-image: url(image@2x.png);
    }
}

Your configuration (any details about your system that you think might be relevant) MacOS Catalina v10.15.4 CodeKit v3.12.1 build 32147 Autoprefixer 9.7.6

bdkjones commented 4 years ago

Thanks for the thorough writeup!

I've been able to reproduce the same result you're getting. However:

1) I am absolutely sure that the correct BrowsersList string is being passed to Autoprefixer. I've logged it in the debugger over here.

2) I am NOT sure that the overrideBrowersList option in Autoprefixer is functioning correctly on their end.

Next:

To test this further, I'd like to take the website version of Autoprefixer out of the equation. There's too many variables there, such as which version of canIUse they're on, etc.

Instead, please install postcss and autoprefixer using npm into your project folder. Then, use this script to run Autoprefixer. You'll need to enter the correct file paths in place of the ${...} placeholders. You can run this script using node -e PATH_TO_SCRIPT:

const autoprefixer = require('autoprefixer');
const postcss = require('postcss');
const fs = require('fs');

process.on('uncaughtException', function (err) {
                process.stderr.write(err.message);
                process.exit(1);
           });

var cssContent = '';
try {
    cssContent = fs.readFileSync(`${INPUT_CSS_FILE_PATH}`, 'utf8');
} catch(e) {
    process.stderr.write('Unable to read the input file.');
    process.exit(100);
}

var plugin = autoprefixer({
    cascade: true,
    grid: false,
    overrideBrowserslist: [">0.2%", "last 2 versions", "Firefox ESR", "not dead", "ie 9-11"]
});

const processor = postcss([plugin]);
processor.process(cssContent,
{
    from: `${INPUT_CSS_FILE_PATH}`,
    to: `${OUTPUT_CSS_FILE_PATH}`,
    map: false
})
.then(result =>
{
    result.warnings().forEach(warn => {
        process.stderr.write(error.toString() + '\n')
    })

    if (result.css)
    {
        try {
            fs.writeFileSync(result.opts.to, result.css, 'utf8');
        } catch(e) {
            process.stderr.write(`Unable to write output CSS to path: ${OUTPUT_CSS_FILE_PATH}`);
            process.exit(206);
        }
    }
}).catch(error =>
{
    if (error.name === 'CssSyntaxError')
    {
        process.stderr.write(error.toString());
        process.exit(1);
    }
    // Otherwise, let our global exception handler catch it.
})
bdkjones commented 4 years ago

Update: It does appear that Autoprefixer's overrideBrowsersList option is broken.

If you use the above script, the default browsersList value is used instead of the one we specify with overrideBrowsersList. But, if you set the BROWSERSLIST environment variable to >0.2%, last 2 versions, Firefox ESR, not dead, ie 9-11 before running the script, Autoprefixer then uses the correct browsers.

I've asked @ai to take a look at this. In the meantime, I'll ship an update to CodeKit that does not rely on the overrideBrowsersList Autoprefixer option. I should have that out in the next day or two.

bdkjones commented 4 years ago

@ai advised me that the overrideBrowsersList option has a lowercase "L": overrideBrowserslist

I have made that change and re-tested but am getting the same result. Setting the Browserslist string as an ENV variable before running the script works. Setting the browserslist using:

overrideBrowserslist: ['>0.2%','last 2 versions','Firefox ESR','not dead','ie 9-11']

does not work.

ai commented 4 years ago

@bdkjones can you remove overrideBrowserslist and create a .browserslistrc config instead?

You may have an Autoprefixer in another tool (for instance, cssnano contains Autoprefixer). We do not recommend to use overrideBrowserslist option, because it will be used only in the single instance of Autoprefixer. In contrast Browserslist config will set the same target browsers to all your tools and guarantee that they work together.

bdkjones commented 4 years ago

I can't use .browserslistrc in this case. CodeKit (my app) coordinates passing the browserslist configuration that the user specifies in the UI to each tool involved in processing (Babel, Autoprefixer, Rollup, etc.) That way, the user doesn't need to mess with config files on disk—they set the option in the app's UI and CodeKit then passes the appropriate string to each tool.

So we still get the benefit of a single browserslist string that applies to all tooling in a project, it's just that we specify that string in a GUI instead of in a config file on disk.

I CAN use the environment variable approach, but I think this overrideBrowerslist option is not working as it should right now. It may be worth looking into it on your end, since the option does exist and is documented.

And, of course, it may still be the case that I'm an idiot and have screwed something up. The test case above is separate from CodeKit and does not rely on anything other than PostCSS, Autoprefixer, and Node.

ai commented 4 years ago

I can't use .browserslistrc in this case

I mean to set .browserslistrc for test purpose. Yeap, another way is to set process.env.BROWSERSLIST.

but I think this overrideBrowerslist option is not working as it should right now

We test our code using overrideBrowerslist. It is extremly unlikely that there is something wrong with it.

Try to run only Autoprefixer in node CLI. You will see that overrideBrowerslist works.

bdkjones commented 4 years ago

@ai Thanks for the link! Looking at those tests gave me an idea and I've solved the problem. I appreciate the help!

@alex-santiago I'll push a new update late tonight that'll resolve this. Thanks for your report.