Css attr error from "postcss-gap" plugin

frugan-dev commented 9 months ago

Hi there,

I'm having this error with Webpack 5:

Error: Module build failed (from ../node_modules/postcss-loader/dist/cjs.js):
SyntaxError
(1:1) from "postcss-gap" plugin: <css input> Unknown word
> 1 | attr(width)/attr(height)
    | ^

These are the main parts of my setup:

package.json

"devDependencies": {
    "flex-gap-polyfill": "^4.2.0",
    "postcss": "^8.4.30",
    "postcss-loader": "^7.1.0",
    "webpack": "^5.84.0",
  },

webpack.config.js

const FlexGapPolyfill = require('flex-gap-polyfill');

module.exports = (env, argv = {}) => {
    ...

    module: {
      rules: [
        {
          test: /\.(sa|sc|c)ss$/,
          use: [

           ....

            {
              loader: 'postcss-loader',
              options: {
                postcssOptions: {
                  plugins: [
                    [
                      require('autoprefixer'),
                    ],
                    [
                      'flex-gap-polyfill',
                      {
                        flexGapNotSupported: '.no-flexgap',
                      },
                    ],
                  ],
                },
              },
            },

           ....
          ],
        },

    ...
};

main.scss

//https://jakearchibald.com/2022/img-aspect-ratio/
//https://www.andreaverlicchi.eu/aspect-ratio-modern-reserve-space-lazy-images-async-content-responsive-design/
img[width][height] {
  height: auto;
  aspect-ratio: attr(width) / attr(height);
}

The error appears only when I enable your plugin, and with the plugin enabled it disappears only by commenting the line aspect-ratio: attr(width) / attr(height); of the scss file.

What could it depend on?

gavinmcfarland commented 9 months ago

Hi @frugan-it. Hmm, this is an odd one. I can't recreate the issue when I create an example repo with webpack and postcss-loader.

This is my webpack.config.js file:

module.exports = {
    // ...
    module: {
        rules: [
            {
                test: /\.css$/i,
                include: path.resolve(__dirname, 'src'),
                use: ['style-loader', 'css-loader', 'postcss-loader'],
            },
        ],
    },
}

Can you provide a repo so I can test?

As far as I know, the plugin shouldn't be doing anything to the property aspect-ratio. So I wonder if something else is tripping it up, and the error about attr(width) / attr(height) is just a red herring.

frugan-dev commented 9 months ago

Hi @gavinmcfarland, thanks for the support!

I created this test repository https://github.com/frugan-it/flex-gap-polyfill-issue-85. If you try to comment these lines https://github.com/frugan-it/flex-gap-polyfill-issue-85/blob/master/webpack.config.js#L187-L192 everything works correctly. I tried to isolate the problem as much as possible, but I don't understand what it could depend on..

gavinmcfarland commented 8 months ago

Thank you! I'll check it out and do some investigating.

gavinmcfarland commented 8 months ago

So, I've looked into it a bit more.

There is a conflict between sass-loader and a dependency my plugin uses called postcss-values-parser.

sass-loader is removing the spaces between attr(width) / attr(height), which then postcss-values-parser trips over because it's expecting a space before and after the /. I don't know which is correct in this case, but that's what's causing the issue.

You can try the following to work around it.

Tell flex-gap-polyfill to ignore processing the rule:

img[width][height]{
  /* added by fgp */
  height: auto;
  aspect-ratio: attr(width) / attr(height);
}

Or you can manually apply the polyfill:

img[width][height]{
  /* added by fgp */
  --fgp-height: var(--element-has-fgp) calc(auto + var(--fgp-gap-row, 0%));
  height: var(--fgp-height, auto);
  aspect-ratio: attr(width, 1) / attr(height, 1);
}

I tried to disable the modification that Sass applies but it seems a little unpredictable. I found some others have had difficulty with it too. https://github.com/webpack-contrib/sass-loader/issues/763

Let me know how you get on.

gavinmcfarland commented 8 months ago

I can probably also release a fix by adding some logic to avoid reading the values like the ones in the aspect-ratio prop as it doesn't need to.

frugan-dev commented 8 months ago

Thanks @gavinmcfarland, sorry for the late reply..

I didn't know you could tell flex-gap-polyfill to ignore some lines, maybe that should be specified in the README? Anyway thank you, now with this comment it seems to work!

gavinmcfarland commented 8 months ago

No worries. I initially never saw the need to ignore certain rules, but I remembered the plugin ignores rules it's already processed, which have a comment of * added by fgp */ in them. I'll add more official support for this when I get chance.

gavinmcfarland / flex-gap-polyfill

Css attr error from "postcss-gap" plugin #85