gavinmcfarland / flex-gap-polyfill

A PostCSS plugin to emulate flex gap using margins
https://gavinmcfarland.github.io/flex-gap-polyfill/
Creative Commons Zero v1.0 Universal
143 stars 6 forks source link

Syntax error from CSS minify tool #43

Closed mashirozx closed 10 months ago

mashirozx commented 2 years ago

image

This is a flex item, I tried to remove the var(--parent-has-fgp), and it just works fine. But I don't know why it generate this var(--parent-has-fgp) at all...

mashirozx commented 2 years ago

Some info:

module.exports = ({ env, options }) => ({
  ...options,
  plugins: [require('autoprefixer')(), require('flex-gap-polyfill')()],
})

"vue": "^3.1.4", "sass": "^1.35.1", "sass-loader": "^12.1.0", "vite": "^2.4.2",

gavinmcfarland commented 2 years ago

Hi,

Are you still having an issue with this? Is it Chrome where the syntax issue is being reported? My suspicion is that Chrome is incorrectly reporting this.

mashirozx commented 2 years ago

Hi! I finally found it was actually a compatibility issue with Vite and esbuild (Vite works depending on esbuild). When esbuild minifying CSS, it shows some of --fgp-* variables are invalid and ignoring them, so the polyfill not work after build.

gavinmcfarland commented 2 years ago

Hmm interesting. If you have a repo you're able to share I can investigate it and try to find a solution. Many thanks.

mashirozx commented 2 years ago

I've created a small demo: https://github.com/mashirozx/flex-polyfill-debug-demo

Some information: HTML and styles here: App.vue

Without polyfill: image

Polyfill on dev server (css not minified): image The gap polyfill works, but seems the width and wrap performance not work as we expect.

Polyfill on build (./dist): image

I unchecked the gap property because what we need is to polyfill it when browser not support it. Problem is here: image

And while yarn build, we can see this: image

gavinmcfarland commented 2 years ago

Thank you, this is really useful. I'll spin this up and take a look.

With regards to the esbuild minifying the CSS I think this is a mistake being made by the minification tool. Although the property may look empty, there is actually a space as a value which is valid CSS. It uses this to toggle between values. We'd have to create a ticket for the esbuild minification to address this.

mashirozx commented 2 years ago

Well, I'm not 100% sure it is the ESbuild's problem (maybe another modules depended by Vite), there may still need more investigation.

And also I found some other strange problem. You can find it in picture 2 above.

image

This occurs on dev server, whose code is not minified, the width is incorrect, do you have any idea on this?

gavinmcfarland commented 2 years ago

Yes regarding the last problem. I had a little play around and it's because of the negative margin on the container and the width being applied is causing the width to be smaller than it is. The answer is to add the column gap to the width using calc but it will need a bit of extra work to make sure this works universally with the polyfill.

gavinmcfarland commented 2 years ago

Hi @mashirozx in your particular case you can try this snippet below:

.flex-box {
    /* Only add to containers with explicit width */
    --fgp-width: var(--has-fgp) calc(170px + var(--fgp-gap-column));
    width: var(--fgp-width, 170px);
}

I'm working on a method to enable this with the polyfill.

mashirozx commented 2 years ago

But when I added this, the polyfills were gone 😢 image

gavinmcfarland commented 2 years ago

Ahh. Sorry my bad. I forgot the plugin looks for properties beginning with --fgp to tell if the polyfill has been applied or not. Just change the prefix or you can remove it entirely, --width will suffice.

gavinmcfarland commented 2 years ago

I've released a fix for the width issue you were encountering. The problem should be resolved in version 4.1.0.

Regarding the syntax error, I believe this is an error with the tool that minimises the CSS because it is removing empty custom properties which are valid CSS.

gavinmcfarland commented 2 years ago

Just did some lite research and I think I've found an option to disable hopefully removing empty properties removeEmpty: true. More information is on the css-clean documentation.

gavinmcfarland commented 10 months ago

I'm closing this for now, as empty properties are needed for the polyfill to work. People can try the suggestion above to see if it resolves the issue.