language-tools icon indicating copy to clipboard operation
language-tools copied to clipboard

Language server CSS: incorrect error for :global(>...) (direct combinator in :global)

Open rixo opened this issue 4 years ago • 2 comments

Describe the bug

In Svelte, it is legal to have a selector starting with a combinator (>, +, or ~) inside the :global(...) modifier, as demonstrated in this REPL.

div :global(> *) { ... }

However in CSS, this is not legal in a pseudo selector (I guess), and so the language server reports a series of parsing errors for this kind of rules.

To Reproduce

Add the following code in a .svelte file in VSCode:

<style>
  :global(> *) {
    color: blue;
  }
</style>

I have also added a failing test on the branch css-global-direct-combinator of my fork.

Expected behavior

No incorrect error reported.

Additional context

I have tracked the problem down to this line in vscode-css-languageservice.

I have confirmed replacing this._parseSelector(false) with this._parseSelector(true) in the case the pseudo selector text is 'global' fixes the problem.

Unfortunately, the function we'd need to adapt is more than buried inside vscode-css-languageservice, so I don't know what should be the path to a proper fix. The more straightforward would probably to add some kind of Svelte support inside vscode-css-languageservice but I'm not sure they'd be willing to do that. I've managed to patch it from the outside, but that's 150 copied line from upstream to apply the 2 line fix...

rixo avatar Dec 09 '21 23:12 rixo

This is unfortunate, and I also don't see a good way out of here without taking on a huge maintance cost. Is it somehow possible to monkey-patch this at runtime? The other possibility would be to try to detect false positive diagnostics by looking at the original code and then filter them out/silence them.

dummdidumm avatar Dec 10 '21 08:12 dummdidumm

Having slept on this, I think the proper way to write this is the following:

div > :global(*) { ... }

Except if I'm mistaken, this is equivalent, but with the advantage that it is valid CSS that is compatible with a normal parser. Also, it is arguably more pleasant syntax.

There is no point for Svelte to support the alternative, invalid CSS, syntax, and I doubt this was intentional in the first place. More like an inadvertent side effect of the implementation I guess.

That and the huge maintenance burden it would mean for language tools... IMO the proper course of action is to deprecate that in Svelte 4 with a compiler error. In effect, this seems like a disturbing precedent for Svelte to break compatibility with standard CSS parsers (especially for no benefit at all, in the case at hand).

For now, I think we can leave things as they are in language tooling. In a sense, this "bug" prevented and will keep preventing users from creating code that would need to be migrated down the road.

rixo avatar Dec 10 '21 13:12 rixo