flex-gap-polyfill icon indicating copy to clipboard operation
flex-gap-polyfill copied to clipboard

Syntax error from CSS minify tool

Open mashirozx opened this issue 3 years ago • 13 comments

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 avatar Jul 28 '21 03:07 mashirozx

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",

mashirozx avatar Jul 28 '21 03:07 mashirozx

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.

gavinmcfarland avatar Jul 30 '21 22:07 gavinmcfarland

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.

mashirozx avatar Jul 30 '21 23:07 mashirozx

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

gavinmcfarland avatar Jul 31 '21 08:07 gavinmcfarland

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

mashirozx avatar Aug 01 '21 03:08 mashirozx

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.

gavinmcfarland avatar Aug 01 '21 15:08 gavinmcfarland

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?

mashirozx avatar Aug 02 '21 03:08 mashirozx

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 avatar Aug 02 '21 10:08 gavinmcfarland

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.

gavinmcfarland avatar Aug 07 '21 14:08 gavinmcfarland

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

mashirozx avatar Aug 08 '21 15:08 mashirozx

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 avatar Aug 10 '21 23:08 gavinmcfarland

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 avatar Aug 15 '21 12:08 gavinmcfarland

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 avatar Aug 20 '21 20:08 gavinmcfarland

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.

gavinmcfarland avatar Aug 24 '23 10:08 gavinmcfarland