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.

Sign up for GitHub

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

Jump to bottom

Scoped variables cause an error when sort-order is enabled #443

Closed
gajus opened this issue Jan 13, 2016 · 13 comments
Closed

Scoped variables cause an error when sort-order is enabled #443

gajus opened this issue Jan 13, 2016 · 13 comments
Labels
category: gpe status: fixed type: bug

Comments

@gajus
Copy link

gajus commented Jan 13, 2016 •
edited by tonyganch
Loading 

.form {
    $foo: #fff;
    $bar: #fff;
}

Produces:

TypeError: a.match is not a function
    at sortLeftovers (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb/lib/options/sort-order.js:244:19)
    at /Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb/lib/options/sort-order.js:334:24
    at Array.sort (native)
    at module.exports.process (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb/lib/options/sort-order.js:325:16)
    at Object.Node.map (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/gonzales-pe/lib/node.js:111:9)
    at /Users/gajus/Documents/dev/gajus/pragmatist/node_modules/gonzales-pe/lib/node.js:117:22
    at Array.forEach (native)
    at Object.Node.map (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/gonzales-pe/lib/node.js:115:22)
    at /Users/gajus/Documents/dev/gajus/pragmatist/node_modules/gonzales-pe/lib/node.js:117:22
    at Array.forEach (native)
    at Object.Node.map (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/gonzales-pe/lib/node.js:115:22)
    at /Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb-core/lib/core.js:132:18
    at Array.forEach (native)
    at Object.processTree (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb-core/lib/core.js:131:19)
    at processString (/Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb-core/lib/core.js:523:22)
    at /Users/gajus/Documents/dev/gajus/pragmatist/node_modules/csscomb-core/lib/core.js:449:38

I did a bit of debugging:

This is sortLeftovers function from sort-order.js that triggers the error.

var sortLeftovers = function(a, b) {
    var prefixes = ['-webkit-', '-moz-', '-ms-', '-o-', ''];
    var prefixesRegExp = /^(-webkit-|-moz-|-ms-|-o-)(.*)$/;

    // Get property name (i.e. `color`, `-o-animation`):
    a = a.node.get(0).get(0).content;
    b = b.node.get(0).get(0).content;


    console.log(a, b);

The output of console.log is:

[ { type: 'ident', content: 'foo', start: { line: 2, column: 6 } } ]
[ { type: 'ident', content: 'bar', start: { line: 3, column: 6 } } ]

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/29843200-scoped-variables-cause-an-error-when-sort-order-is-enabled?utm_campaign=plugin&utm_content=tracker%2F214563&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F214563&utm_medium=issues&utm_source=github).
@gajus gajus changed the title Scoped variables produce an error when sort-order is enabled Scoped variables cause an error when sort-order is enabled Jan 13, 2016
@nnmrts
Copy link

nnmrts commented Dec 5, 2017

For me this isn't fixed. Still get this issue when I use variables or at-rules. :/

@nnmrts
Copy link

nnmrts commented Dec 7, 2017

Windows 10, vscode insider build and stable build
csscomb 4.2.0

config:

{
	"always-semicolon": true,
	"color-case": "lower",
	"block-indent": "\t",
	"color-shorthand": false,
	"element-case": "lower",
	"eof-newline": true,
	"leading-zero": true,
	"quotes": "double",
	"sort-order-fallback": "abc",
	"space-before-colon": "",
	"space-after-colon": " ",
	"space-before-combinator": " ",
	"space-after-combinator": " ",
	"space-between-declarations": "\n",
	"space-before-opening-brace": " ",
	"space-after-opening-brace": "\n",
	"space-after-selector-delimiter": "\n",
	"space-before-selector-delimiter": "",
	"space-before-closing-brace": "\n",
	"strip-spaces": true,
	"unitless-zero": true,
	"vendor-prefix-align": true,
        "sort-order": [
                // ....
        ]
}

For me, csscomb stops working if the scss file has an @-rule or a scoped variable in it:

// doesn't work
#header {
        width: 100%;
        @include blackBg;
}

// doesn't work
#header {
        width: 100%;
        $headerHeight: 64px;
}

// does work
#header {
        width: 100%;
        height: $headerHeight;
}

// does work
#header {
        width: 100%;
        #{$bg}-color: blue
}

In the cases it doesn't work, csscomb throws me an error:

TypeError: a.match is not a function

When I remove my sort-order property from the config, everything works fine. :)

@jdalton
Copy link
Contributor

jdalton commented May 6, 2019

This should be fixed in the latest release now that the sortLeftovers uses

a = a.node.first().first().content;
b = b.node.first().first().content;

@jdalton jdalton closed this as completed May 6, 2019
@JiniHendrix
Copy link

still getting this issue

@jdalton
Copy link
Contributor

jdalton commented May 15, 2019

Hi @JiniHendrix!

Can you create a small repro repo that demonstrates the issue for me to investigate.

@JiniHendrix
Copy link

hey @jdalton here's an example repo https://github.com/JiniHendrix/csscomb-sort-break

thanks!

@jdalton jdalton reopened this May 15, 2019
@jdalton
Copy link
Contributor

jdalton commented May 15, 2019

Thank you @JiniHendrix!

Yep, the issue still persists. Let's fix it together!
Would you be up for creating a failing test case in a PR that I can write a patch against?

@JiniHendrix
Copy link

ill give it a shot 👍

@jdalton
Copy link
Contributor

jdalton commented May 15, 2019

Rock! The issue is that because left-overs can be a variety of things, one of those being a Variable node which has content as an array (instead of a string like the Ident node).

@JiniHendrix
Copy link

@jdalton looks like there already is a test for it, and it's passing. may need to refactor the tests.

test/options/sort-order/process/test.js line 413

@JiniHendrix
Copy link

dont really have the time for it unfortunately, seems like you know how to fix the issue though?

@jdalton
Copy link
Contributor

jdalton commented May 16, 2019 •
edited
Loading

If you're up for the refactor that would be appreciated too : )

Ah okay, sorry. I posted my initial reply before the newer comments popped in. No worries. Thanks for pin pointing the existing test!

Update:

Patch PR started at #609.

jdalton added a commit that referenced this issue May 17, 2019
@jdalton
Merge pull request #609 from jdalton/scope-vars
37572bb 
Fix error when sorting leftover variable definitions. [closes #443]
jdalton added a commit that referenced this issue May 17, 2019
@jdalton
Fix error when sorting leftover variable definitions. [closes #443]
1f27f9e
@JiniHendrix
Copy link

hey @jdalton thanks for fixing the issue. any idea when this will be published?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
category: gpe status: fixed type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jdalton @gajus @haoqunjiang @JiniHendrix @nnmrts