error with star-prefixed css properties

[dominikg]

const css = require('@parcel/css');

css.transform({
    filename: 'style.css',
    code: Buffer.from('.clearfix { *zoom: 1; }'),
    minify: true
});

results in

SyntaxError: Unexpected token Delim('*')
    at Object.<anonymous> (/home/dominikg/develop/vite-ecosystem-ci/parcel.cjs:3:5)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47 {
  fileName: 'style.css',
  source: '.clearfix { *zoom: 1; }',
  loc: { line: 1, column: 13 }
}

found via css-minification-benchmark. GoalSmashers/css-minification-benchmark#101

Some of their test data is old and still has them: https://github.com/GoalSmashers/css-minification-benchmark/blob/e1d52eaea8e1107e95ea06f5cc085227cd36b2c0/data/gumby.css#L1216

iirc they were used to add special treatment for ancient IE

https://stackoverflow.com/questions/1667531/what-does-a-star-preceded-property-mean-in-css

[devongovett]

Hmm not sure what to do about this. That's invalid CSS according to the spec. Browsers would ignore it, but we raise errors at build time instead so you can catch mistakes. Maybe we can have some kind of exception for this pattern, though not sure how much it's used in more modern code.

[dominikg]

In general i think it's OK and helpful to throw for out-of-spec input.

But if browsers are able to ignore it a case could be made that other tools should be as well.
So an option to treat these errors as warnings instead could help to avoid similar issues in the future.

The errors in the benchmark could be resolved on their end by better error handling or updating input data to remove outdated css.

[devongovett]

So I guess we have two options:

1. Completely ignore these properties. They'd get removed in the output, just like modern browsers do. But this would drop IE 6/7 support (seems fine these days?).
2. Try to preserve them unchanged.

Both might be a little challenging without modifying the underlying cssparser crate, but I'll give it a shot.

[evantbyrne]

Something to take into account is that Microsoft will have dropped all IE support by June 15, 2022, with only IE 11 as of writing receiving any support. Allowing certain CSS errors to support CSS hacks in long since abandoned browsers may be unexpected behavior for most users.

[onigoetz]

I think I have seen another case yesterday .b { color: red !ie } which throws a SyntaxError: Unexpected token Delim('!') (I can do a separate bug report for that btw)

While I understand those are hacks for old browsers, they are none-the-less used in the wild and sometimes it's an import that people have no direct control over. I think it would ease adoption if those were preserved unchanged (or could be removed automatically if minification specifies that IE isn't supported)

[dominikg]

the benchmark has removed the files with invalid syntax and added @parcel/css

https://goalsmashers.github.io/css-minification-benchmark/

The performance numbers are crushing across the board 🚀
Filesize is also great, though a bit less than what https://github.com/parcel-bundler/parcel-css#benchmarks suggests

The original trigger for this issue is resolved and i believe/hope that users don't run into this or rather appreciate the hint and fix their css.
I'd suggest to close this as known issue and only take action once real-world problems show up that warrant support.

[devongovett]

In regards to file size, one difference is that other benchmark is not setting any browser targets so Parcel CSS cannot remove vendor prefixes etc., whereas in the benchmark here it does.

Another thing to keep in mind is that many of the other tools listed there perform unsafe transformations such as merging non-adjacent rules. So they may appear to produce smaller style sheets but may also change the behavior. Parcel CSS only does safe transforms.

[evantbyrne]

It should additionally be noted that CSS hacks were never necessary to target IE 6 and 7 anyways. IE conditional comments were standard practice for selectively applying styles to those browsers back when I worked on those kinds of projects, so I recommend that in lieu of introducing inconsistent error handling. I second closing this ticket until someone comes back with a real world use case.

[onigoetz]

@evantbyrne yes IE conditional comments do exist, but I'm not sure a lot of libraries out there provide a "ie7.css" and an "ie11.css" file. It's definitely more interesting to have a single file where all the browser-specific adjustments can coexist.

[dominikg]

Projects still requiring these specific IE hacks are very unlikely to switch to parcel-css for their css minification needs.

There is a possibility that there are other uses that could warrant a mode in parcel-css where invalid rules are tried to be kept, but that should be discussed when presented rather than investing resources and complicating the internal structure now.

regarding file size discrepancy in benchmark:
Imho you're doing an apples to oranges comparison when you enable vendor-prefix stripping in parcel-css.
I love that feature and get that you want to highlight it, but the way the results are currently presented in the README is misleading.

[onigoetz]

Having the need to support old browsers isn't necessarily correlated with the tooling you are using around.
Where I work we were able to drop IE 11 only last september, but we used modern tooling long before that.

So that doesn't exactly apply to me, but if parcel/css came out last year I would definitely have wanted to use it for those builds that ship for IE 11, it's not because I have to support old browsers that I don't want fast tools.

[devongovett]

Imho you're doing an apples to oranges comparison when you enable vendor-prefix stripping in parcel-css.

I don't agree with that. Should we disable all optimizations that other minifiers don't perform? Should we disable optimizations in other minifiers that Parcel CSS doesn't perform?

[evantbyrne]

@onigoetz Unfortunately there's a tradeoff and right now the reported issue is a hypothetical problem. Stopping on syntax errors is a valuable feature generally. To support IE 6/7 specific CSS parsing, then parcel-css would either need some kind of IE 6/7 mode or to regress syntax validation across the board. That's the reasoning for my recommendation. Best of luck working with IE 11, it wasn't that long ago I had to support that version as well so I know your pain!

[onigoetz]

@evantbyrne In understand the tradeoff, and I understand that it would be complicated to add that support. I would then recommend to add a note in the documentation that IE hacks aren't supported as this might surprise people.

As said, they might have that not only in their own code but in libraries that they might import and have no control over.

[thesoftwarephilosopher]

It's not a hypothetical problem. I got here by googling "parcel ignore css syntax error" because a CSS lib I'm trying to import has an asterisk'd property. I just need to get this thing working with Parcel as if it were in the browser, and move on.

[thesoftwarephilosopher]

In the meantime, is there a way to hook into Parcel's workflow so that I can just .replace() the *display into display so Parcel can do its thing?

[niekcandaele]

I was able to get around this by creating a custom transformer plugin that removes any asterisk prefixed css rules. I don't plan to support old browsers so this is okay for my usecase.

const { Transformer } = require('@parcel/plugin');

module.exports = new Transformer({
  async transform({ asset, config, logger }) {
    const source = await asset.getCode();
    const replaced = source.replaceAll(/^\s*\*.+$/mg, '');
    asset.setCode(replaced)
    return [asset];
  },
});

This changes my error message, now I get

@parcel/transformer-css: Invalid token in pseudo element: WhiteSpace(" ")

  /home/xxx/node_modules/datatables.net-dt/css/jquery.dataTables.css:349:15
    348 |   /* Chrome,Safari4+ */
  > 349 |   background: -webkit-linear-gradient(top, white 0%, #dcdcdc 100%);
  >     |               ^
    350 |   /* Chrome10+,Safari5.1+ */
    351 |   background: -moz-linear-gradient(top, white 0%, #dcdcdc 100%);

Not sure where to go from here but sharing in case this helps someone else :)

[devongovett]

As of the above commit, there is a new errorRecovery option which can be enabled. This ignores invalid rules and declarations rather than erroring (just like a browser would). They are omitted from the output, and a warning is emitted instead. Once this is released, I will work on exposing it as a config option in Parcel as well. Hopefully this allows people to work around issues in libraries more easily.

[hamidrezakp]

Hello everyone,
I have a use case that needs to parse and minify css files from users and i have no control over css files they give me.

In this scenario, ignoring or returning error on invalid css is not an option, i have to just preserve invalid rules and also support * prefixes.

I have made a little change in rust-cssparser crate (servo/rust-cssparser#388) that lightningcss uses, it works but is not a clean solution.
Just wanted to let you know if you need this, just replace it in your codebase like this:

Cargo.toml:

[patch.crates-io]
cssparser = { git = "https://github.com/hamidrezakp/rust-cssparser/", rev = "v0.33.0" }