Error with scss nested property syntax

[s10wen]

With the following code (using gulp-stylelint and syntax: 'scss'):

.test {
  margin: {
    left: auto;
    right: auto;
  }
}

I get the error:

events.js:141
      throw er; // Unhandled 'error' event
      ^
Error: Expected pseudo-class or pseudo-element
    at new error (…/node_modules/postcss-selector-parser/dist/processor.js:27:23)
    at Parser.error (…/node_modules/postcss-selector-parser/dist/parser.js:218:15)
    at Parser.pseudo (…/node_modules/postcss-selector-parser/dist/parser.js:316:25)
    at Parser.parse (…/node_modules/postcss-selector-parser/dist/parser.js:533:22)
    at Parser.loop (…/node_modules/postcss-selector-parser/dist/parser.js:501:18)
    at new Parser (…/node_modules/postcss-selector-parser/dist/parser.js:93:21)
    at Processor.process (…/node_modules/postcss-selector-parser/dist/processor.js:24:21)
    at Plugin.<anonymous> (…/node_modules/stylehacks/dist/plugins/bodyEmpty.js:36:67)
    at Plugin.<anonymous> (…/node_modules/stylehacks/dist/plugin.js:40:29)
    at …/node_modules/postcss/lib/container.js:81:26

Wondering if anyone can point me in the right direction, of why this might be happening.

[m-allanson]

Is margin: { valid scss?

[s10wen]

I'm not a fan of doing this, but I believe it is:
http://codepen.io/s10wen/pen/OXPGVE

Compiles to:

.test {
  margin-left: auto;
  margin-right: auto;
}

[m-allanson]

Ah ok...

It looks like it's an issue with postcss-selector-parser: postcss/postcss-selector-parser#64

[s10wen]

yup, looks like the same issue, I'm not sure of the correct terminology for code like:

.test {
  margin: {
    left: auto;
    right: auto;
  }
}

but it would be nice to have a rule, that would check for this, as I'd like to avoid it in the codebase I'm working on.

[s10wen]

or another suggestion, if possible, that would be useful, is if the error code stated the filename / line / character where the error occurred.

[m-allanson]

Rule requests I'll leave to @davidtheclark or @jeddy3 to comment on. Although I think recently they're aiming to encourage community members to create stylelint plugins for rules that handle non-standard css syntax.

[s10wen]

ok, thanks for the feedback, yeah, appreciate this isn't a css issue.

[s10wen]

In case it helps anyone else that ends up here, there's an issue for this on Stack Overflow as well:
http://stackoverflow.com/questions/35798373/posibility-for-nested-properties-in-stylelint-in-gulp

[davidtheclark]

At some point recently (6.4.0, I believe) we tried to catch errors thrown by postcss-selector-parser and present them as a PostCSS warning that would say Cannot parse selector along with the position of the error.

It looks from your error like the stylehacks rule, no-browser-hacks, has not yet conformed to the new order and needs to be modified to provide the same UX. So I'm going to re-open this.

[davidtheclark]

In fact, this raises another issue with the use of stylehacks: Our internal utils would help us ignore nonstandard selectors like this (ending in a :), but stylehacks doesn't have the same safeguards. We might need to figure out a way to make stylehacks ignore this selector altogether ...

[davidtheclark]

Looked into this some. Probably a huge pain, so I'm just going to suggest that you disable that rule for that line, and move on. If somebody finds this issue later and wants to really tackle the problem, go for it.

[jeddy3]

Our internal utils would help us ignore nonstandard selectors like this (ending in a :), but stylehacks doesn't have the same safeguards.

For anyone wishing to tackle this upstream in stylehacks, perhaps start with adding this safeguard to each of stylehacks's plugin. And then seeing what mileage that gets you.

but it would be nice to have a rule, that would check for this [use of nested properties], as I'd like to avoid it in the codebase I'm working on.

@s10wen That's a great suggestion for the stylelint-scss plugin pack developers. I recommend creating an issue in their repo for a new rule called declaration-nested-properties: "always"|"never", which would require or disallow allow the use of nested properties within declarations. I'm sure they'd welcome you contributing the rule too.

@s10wen As an aside, one of the advantages of crafting your own CSS Processor with PostCSS is that you don't need to rely on linting to disallow language features. Instead, you simply don't use the postcss-nested-props PostCSS plugin within your pipeline, which makes the feature unavailable to your team. Powerful stuff right there :)

[s10wen]

@jeddy3 cheers, done stylelint-scss/stylelint-scss#44

For anyone else who happens to end up here, another solution is to simply update the code:

Before:

.test {
  margin: {
    left: auto;
    right: auto;
  }
}

After:

.test {
  margin-left: auto;
  margin-right: auto;
}

I personally much prefer the After approach (especially as I also use Emmet).

[dryoma]

Hey guys,

There is a discussion right now in a PR to postcss-scss about supporting nested properties. I believe stylelint would benefit from that as well, so I'd really like to hear your opinions on how exactly this should be implemented.

As a breif intro into the discussion: at the moment nested props groups are treated as Rules and are not distinguished at all. The options are 1) Adding properties to such Rule objects (like prop, value, isNested and adding some raws); that way margin: 0px will still be selector, and checking stuff like if there is a space before a colon will require adding more checks. 2) Making margin: 0px { left: 0; } a Declaration; this will be a breaking change as it could break some hacks around wokring with the current Rule-based parsing.

Any thoughts are welcome in the PR thread.

Thanks!

[davidtheclark]

I think probably the easiest thing for stylelint would be if the nested declarations were exploded into a set of regular declaration nodes with selectors matching what the compiled selector should be. No idea if that's good for other tools, though!

[dryoma]

@davidtheclark thanks for your comment! would you mind leaving one in the PR thread? The thing is I agree that it should be a Declaration being extended, and the author doesn't, so I was hoping other devs could give some pros or cons from their points of view.

As for the scheme you described: wouldn't it mess declaration-block-properties-order that would be trying to sort the nested prop parts together with usual props? I was guessing stylelint might want to declaration-block-properties-order to consider the root prop of the nested group (margin, background, etc), and rules like declaration-colon- to actually not skip nested props.