Problems on parsing root mixins with parameters with Stylelint

[diagramatics]

I managed to scope this problem out to a compatibility issue with Stylelint. This is the code:

.foo() {
    color: #fff;
}

This works with Stylelint. However if I use parameters:

.foo(@bar) {
    color: @bar;
}

This immediately returns an error:

TypeError: this[node.type] is not a function
    at Stringifier.stringify (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:33:24)
    at stringify (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringify.js:14:9)
    at AtRule.toString (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/node.js:97:9)
    at Array.toString (native)
    at Stringifier.atrule (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:74:35)
    at Stringifier.stringify (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:33:24)
    at Stringifier.body (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:93:18)
    at Stringifier.root (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:37:14)
    at Stringifier.stringify (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringifier.js:33:24)
    at stringify (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/stringify.js:14:9)

I've tested this on both gulp-postcss and PostCSS's Node API and both returns the same thing when Stylelint is added to the mix. mixinsAsAtRules and innerMixinsAsRules is already set to true.

Also if you change the mixin from a root mixin to be scoped inside another block, the error goes away.

[diagramatics]

Debugged this and found out that the Stringifier is trying to stringify the mixin-params. I'm pretty sure the mixin params shouldn't be in this stringifying loop? Not really understanding the innards of PostCSS.

[webschik]

@diagramatics , thank you. I will check it asap

[diagramatics]

I've been doing more debugging about how the code works and how postscss-scss handles mixins. If mixinsAsAtRules is activated would it be right if the params are not parsed as an array of mixin-params but as a string parsed from after the .? Feels very undescriptive to what the syntax of mixins in LESS is though.

[shellscape]

I'm a little confused as to why postcss-less cares about the params at all.
It's a syntax for the parser after all, if they exist, it should be part of
the tree.

On Sat, Mar 26, 2016 at 12:44 PM, Steven Sinatra notifications@github.com
wrote:

I've been doing more debugging about how the code works and how
postscss-scss handles mixins. If mixinsAsAtRules is activated would it be
right if the params are not parsed as an array of mixin-params but as a
string parsed from after the .? Feels very undescriptive to what the
syntax of mixins in LESS is though.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#25 (comment)

[webschik]

@diagramatics, do you use version 0.5.0? I've tried it - works well:

const root = parse('.foo(@bar) {color: @bar;}', {mixinsAsAtRules: true, innerMixinsAsRules: true});

[diagramatics]

@webschik Yeah it does. I've checked already and the issue only happens when you add Stylelint into the plugin list.

[webschik]

@diagramatics, please, provide a full example of your code.

[diagramatics]

Right, sorry for not providing this way earlier.

const fs = require('fs');
const postcss = require('postcss');
const syntax = require('postcss-less');
const reporter = require("postcss-reporter")
const stylelint = require("stylelint")
const postcssParserOptions = {
    syntax: syntax,
    mixinsAsAtRules: true,
    innerMixinsAsRules: true,
    from: './test.less'
};
const css = fs.readFileSync('./test.less', 'utf-8');

postcss([
  stylelint({
  }),
  reporter({ clearMessages: true }),
]).process(css, postcssParserOptions).then(function (result) {
}).catch(err => console.error(err.stack));

[webschik]

@diagramatics, thank you. I've fixed it in 0.6.0.

[webschik]

@shellscape, @patsissons added the parsing of parameters for possibility to process them too (for example validate params in linter).
@patsissons, I move mixin-param type to lessType property as we use it for mixins.

[diagramatics]

Awesome, thank you for all the effort you've put in this!

[patsissons]

I can't believe I didn't run into this earlier in testing. I'll add a new test this morning to stylelint to capture this issue.

[diagramatics]

I've tried this again in the same configuration. Seems like the output is still causing compatibility issues with Stylelint.

TypeError: source.substr is not a function
    at checkChar (/Volumes/g/repos/gaf-stylelint/node_modules/stylelint/dist/utils/styleSearch.js:57:16)
    at exports.default (/Volumes/g/repos/gaf-stylelint/node_modules/stylelint/dist/utils/styleSearch.js:168:17)
    at exports.default (/Volumes/g/repos/gaf-stylelint/node_modules/stylelint/dist/utils/cssFunctionArguments.js:8:29)
    at checkAtRuleParams (/Volumes/g/repos/gaf-stylelint/node_modules/stylelint/dist/rules/function-url-quotes/index.js:75:39)
    at check (/Volumes/g/repos/gaf-stylelint/node_modules/stylelint/dist/rules/function-url-quotes/index.js:56:9)
    at /Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/container.js:133:28
    at /Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/container.js:73:26
    at Root.each (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/container.js:60:22)
    at Root.walk (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/container.js:72:21)
    at Root.walkAtRules (/Volumes/g/repos/gaf-stylelint/node_modules/postcss/lib/container.js:131:25)

I've debugged this and source.substr() is being tried on an AtRule object. The checkChar loop is being executed with the source being always a String.

I've also noticed that the root source output Stylelint is parsing has this:

@foo @bar {
    color: @bar;
}

Is the lack of parentheses on the parameters (@foo (@bar)) intended?

[webschik]

@diagramatics, thank you! I with @patsissons will think how to resolve this issue from our side.

[patsissons]

i think the main issue is that we really need to scale back the parsing when we're interacting with stylelint. when postcss is processing the stylelint plugin it doesn't use syntax extensions so we have to make sure the nodes that are created from less specific syntax are simplified to a rule with no further children.

So that means we would want the above example to be parsed to something like:

atrule: .foo
  rule: (@bar) { color: @bar)

By avoiding further child nodes (i.e. params, or other blocks) we allow the default postcss stringifier to just emit the raw less syntax.

[webschik]

@patsissons, we should decide to use only PostCSS API and nodes with skipping of some LESS specific nodes, or extend PostCSS API and provide custom nodes for LESS syntax. Please, look at discussion here.

I like the second variant, but as I see it can brake 3rd-party libraries, as Stylelint, that have strict addiction on PostCSS API and PostCSS node types as AtRule.

[shellscape]

elaborating on my opinion on the PR comment @webschik linked to there... if the goal of postcss-less is not to provide a LESS syntax for PostCSS to the rest of the PostCSS community, but rather to provide a means to allow Stylelint to operate on LESS files, then the project should be renamed and the description changed accordingly - something like stylelint-postcss-less to scope it properly.

Since you've chosen the name postcss-less (which follows postcss-scss), and the description of the project is that it's a syntax for PostCSS, you're going to have a lot of people in the PostCSS community (all of the maintainers of Lesshint as well) looking to this as an actual and proper LESS syntax for PostCSS - something that the project is already departing from. A proper syntax will follow the LESS spec and allow far more than Stylelint compatibility.

Food for thought.

[patsissons]

I was about to suggest the same thing. We need a stylelint focused fork of postcss-less going forward. It just makes more sense because I would like to support the full less syntax here but that means either waiting for stylelint to properly handle things or adding in compatibility hacks as we have been doing.

[webschik]

@shellscape, @patsissons, thank you, guys!
I would like to support full LESS syntax in postcss-less parser and I'm going to extend the parser and remove all hacks for Stylelint some of these days.

[webschik]

@diagramatics, after discussion with @patsissons we decided that Stylelint can't process current syntax tree from postcss-less, because it needs some specific rules and code. @patsissons is going to focus on special parser for Stylelint.

[diagramatics]

Right, I see. Do let me know if the effort has already begun as we are focused on having a LESS parser for Stylelint at the moment so we can help out. Thanks for all the help!