Skip to content
New issue

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

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

Already on GitHub? # to your account

v2.1.16 — TypeError at sorter.js:12:134 #439

Closed
branneman opened this issue Dec 2, 2015 · 13 comments
Closed

v2.1.16 — TypeError at sorter.js:12:134 #439

branneman opened this issue Dec 2, 2015 · 13 comments

Comments

@branneman
Copy link

When running gulp sassdoc, with this gulpfile:

var gulp    = require('gulp');
var sassdoc = require('sassdoc');

gulp.task('sassdoc', function() {
    gulp.src(['./src/static/scss/**/*.scss', './src/components/**/*.scss'])
        .pipe(sassdoc({ dest: './dist/docs/sassdoc' }))
});

This is the error I get:

/Users/<redacted>/node_modules/sassdoc/dist/sorter.js:12
    return compare(a.group[0].toLowerCase(), b.group[0].toLowerCase()) || compare(a.file.path, b.file.path) || compare(a.context.line.start, b.context.line.start);
                                                                                                                                     ^

TypeError: Cannot read property 'start' of undefined
    at /Users/<redacted>/node_modules/sassdoc/dist/sorter.js:12:134
    at Array.sort (native)
    at Object.sort [as default] (/Users/<redacted>/node_modules/sassdoc/dist/sorter.js:11:15)
    at Parser.postProcess (/Users/<redacted>/node_modules/sassdoc/dist/parser.js:70:31)
    at DestroyableTransform.flush [as _flush] (/Users/<redacted>/node_modules/sassdoc/dist/parser.js:146:21)
    at DestroyableTransform.<anonymous> (/Users/<redacted>/node_modules/sassdoc/node_modules/readable-stream/lib/_stream_transform.js:135:12)
    at DestroyableTransform.g (events.js:260:16)
    at emitNone (events.js:72:20)
    at DestroyableTransform.emit (events.js:166:7)
    at finishMaybe (/Users/<redacted>/node_modules/sassdoc/node_modules/readable-stream/lib/_stream_writable.js:371:12)

I'm using sassdoc version 2.1.16. I can provide additional debug info if that's required.

@KittyGiraudel
Copy link
Member

I'm using sassdoc version 2.1.16. I can provide additional debug info if that's required.

Would be nice. :)

@branneman
Copy link
Author

What kind of debug info exactly do you need? I added { verbose: true } to the options, but there's no difference in output.

I've added gulp-debug to my pipe:

var debug = require('gulp-debug');
gulp.task('sassdoc', function() {
    gulp.src(['./src/static/scss/**/*.scss', './src/components/**/*.scss'])
        .pipe(debug())
        .pipe(sassdoc({ dest: './dist/docs/sassdoc' }))
});

But that doesn't give me any other useful info:

$ gulp docs-sassdoc
[13:06:09] Using gulpfile ~/<redacted>/gulpfile.js
[13:06:09] Starting 'docs-sassdoc'...
[13:06:09] Finished 'docs-sassdoc' after 83 ms
[13:06:09] gulp-debug: src/static/scss/all.scss
[13:06:09] gulp-debug: src/static/scss/common/_colors.scss
[13:06:09] gulp-debug: src/static/scss/common/_grid.scss
[13:06:09] gulp-debug: src/static/scss/common/_typography.scss
[13:06:09] gulp-debug: src/static/scss/util/_position.scss
[13:06:09] gulp-debug: src/static/scss/vendor/_normalize.scss
[13:06:09] gulp-debug: src/components/nav/nav.scss
[13:06:09] gulp-debug: 7 items
/Users/<redacted>/node_modules/sassdoc/dist/sorter.js:12
    return compare(a.group[0].toLowerCase(), b.group[0].toLowerCase()) || compare(a.file.path, b.file.path) || compare(a.context.line.start, b.context.line.start);
                                                                                                                                     ^

TypeError: Cannot read property 'start' of undefined
    at /Users/<redacted>/node_modules/sassdoc/dist/sorter.js:12:134
    at Array.sort (native)
    at Object.sort [as default] (/Users/<redacted>/node_modules/sassdoc/dist/sorter.js:11:15)
    at Parser.postProcess (/Users/<redacted>/node_modules/sassdoc/dist/parser.js:70:31)
    at DestroyableTransform.flush [as _flush] (/Users/<redacted>/node_modules/sassdoc/dist/parser.js:146:21)
    at DestroyableTransform.<anonymous> (/Users/<redacted>/node_modules/sassdoc/node_modules/readable-stream/lib/_stream_transform.js:135:12)
    at DestroyableTransform.g (events.js:260:16)
    at emitNone (events.js:72:20)
    at DestroyableTransform.emit (events.js:166:7)
    at finishMaybe (/Users/<redacted>/node_modules/sassdoc/node_modules/readable-stream/lib/_stream_writable.js:371:12)

I'm running Node.js version 5.0.0 on OSX 10.11.1.

@branneman
Copy link
Author

The problem seems to be solved by downgrading to Sassdoc v2.1.15, even with Node.js v5.1.0.

So there's a difference between 2.1.15 and 2.1.16.

@branneman branneman changed the title Node.js TypeError at sorter.js:12:134 v2.1.16 — TypeError at sorter.js:12:134 Dec 2, 2015
@valeriangalliat
Copy link
Member

Related to #438 maybe?

Ok found it.

Since SassDoc/scss-comment-parser#21, the code that adds the line is not executed for CSS rules.

Not sure how to fix this since we don't have startIndex and endIndex in the else, and I don't have time to figure this out and test it right now.

I'm downgrading scss-comment-parser meanwhile this issue is fixed on scss-comment-parser side.

valeriangalliat added a commit that referenced this issue Dec 2, 2015
@valeriangalliat
Copy link
Member

Temporarily fixed with 2.1.17, which is basically the same as using 2.1.15... (but at least won't affect new installs).

@carljm
Copy link
Contributor

carljm commented Dec 14, 2015

Sorry for this regression. I did not realize that the line info was required by SassDoc. I will look into a fix to provide it in scss-comment-parser.

@valeriangalliat
Copy link
Member

@carljm That would be awesome, thank you! No hurry though, we can't say repos are active currently, and the downgrade fix can live for a while. :D

@branneman
Copy link
Author

This would be preventable with the right tests, have you thought about adding them?

@pascalduez
Copy link
Member

Agreed, the fact that this totally sneaked through our tests is the sign we should give it a more serious though and planning. Sadly we are all lacking free time lately.

@carljm
Copy link
Contributor

carljm commented Dec 15, 2015

When I fix scss-comment-parser, I will also see about possibly adding a test to sassdoc that would have caught the regression.

@carljm
Copy link
Contributor

carljm commented Dec 17, 2015

See SassDoc/scss-comment-parser#23 and #442

@pascalduez
Copy link
Member

Should be fixed as of sassdoc@2.1.19, thanks a lot @carljm for the fixes!

@carljm
Copy link
Contributor

carljm commented Dec 22, 2015

👍 thanks for the review and merge!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

No branches or pull requests

5 participants