Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to parse css variables without trailing semicolon #250

Closed
hucsmn opened this issue Dec 4, 2020 · 7 comments
Closed

Failed to parse css variables without trailing semicolon #250

hucsmn opened this issue Dec 4, 2020 · 7 comments

Comments

@hucsmn
Copy link

hucsmn commented Dec 4, 2020

Input:

.item {
  --min-height: 66px
}

.item::placeholder {
  font-size: 24px;
}

Produces:

{
  value: '.item',
  root: null,
  parent: null,
  type: 'rule',
  props: [ '.item' ],
  children: [
    {
      value: '--min-height:66px}.item::placeholder{font-size:24px;',
      root: [Circular],
      parent: [Circular],
      type: 'decl',
      props: '--min-height:66px}.item::placeholder{font-size',
      children: '24px',
      line: 6,
      column: 19,
      length: 46,
      return: ''
    }
  ],
  line: 1,
  column: 8,
  length: 0,
  return: ''
}

This corner case is reproducable in both 4.0.3 and 4.0.6.

Interestingly, there was no problem if simply replace --min-height with min-height:

{
  value: '.item',
  root: null,
  parent: null,
  type: 'rule',
  props: [ '.item' ],
  children: [
    {
      value: 'min-height:66px;',
      root: [Circular],
      parent: [Circular],
      type: 'decl',
      props: 'min-height',
      children: '66px',
      line: 3,
      column: 2,
      length: 10,
      return: ''
    }
  ],
  line: 1,
  column: 8,
  length: 0,
  return: ''
}
@Andarist
Copy link
Collaborator

Andarist commented Dec 4, 2020

The reason is that we are turning off } handling when consuming the value of a CSS variable:
https://github.com/thysultan/stylis.js/blob/457a9fd29eb75cfe58002937a3b3924d62601d48/src/Parser.js#L66
which is done to handle this bizarre edge case:
https://github.com/thysultan/stylis.js/blob/457a9fd29eb75cfe58002937a3b3924d62601d48/test/Parser.js#L829

Not sure how to best approach fixing that in a minimal amount of code - I'm crying when thinking about those CSS variable edge cases.

@thysultan
Copy link
Owner

I've always had a (╯°□°)╯︵ ┻━┻ vibe with those css variables edge cases.

@layershifter
Copy link

Just met the same issue, the easiest repro is below:

Input

.foo { --bar: var(--baz) }

Output

.foo {
--bar:var(--baz)}
}
	
;
}

@thysultan
Copy link
Owner

thysultan commented Mar 20, 2021

Although it is useable if you just add an explicit semicolon. @Andarist I think this issue is more important as is mentioned in frontity/frontity#753 than the css-variable edge case of --at-keyword-unknown-block: @foobar {}; so we should revert to not supporting that?

@thysultan
Copy link
Owner

Also i wonder what does the css spec say about h1{--example: {} is it h1{--example:{;} or h1{--example:{} the later of which is an unterminated block.

@Andarist
Copy link
Collaborator

@Andarist I think this issue is more important as is mentioned in frontity/frontity#753 than the css-variable edge case of --at-keyword-unknown-block: @foobar {}; so we should revert to not supporting that?

Agreed that this is more important. Question is - is Stylis aiming to parse all valid CSS or are we not caring about edge cases like this? I'm fine with both - if we don't care then this should be mentioned in the docs. It's a fair tradeoff but has to be acknowledged.

Also i wonder what does the css spec say about h1{--example: {} is it h1{--example:{;} or h1{--example:{} the later of which is an unterminated block.

Quite frankly - I don't want to dive deep figuring this out from the spec. I've checked 2 things though:

  1. Prettier claims that:

SyntaxError: CssSyntaxError: Unclosed block (1:1)

  1. Chrome allows this:
var s = document.createElement('style')
document.head.append(s)
s.sheet.insertRule(`h1{--example: {}`)
s.sheet.cssRules[0].cssText // "h1 { --example: {}; }"

@thysultan
Copy link
Owner

A better test for that would be

var s = document.createElement('style')
document.body.append(s)
s.innerHTML = 'body{--example:{} body{background:red;}'

Which results in

body {
  --example: {} body{background:red;};

Whereby body{background:red;} ends up being part of the --example variable and not another rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants