SassDoc / sassdoc

Release the docs!
http://sassdoc.com
MIT License
1.41k stars 56 forks source link

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

Closed branneman closed 8 years ago

branneman commented 8 years ago

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 commented 8 years ago

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

Would be nice. :)

branneman commented 8 years ago

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 commented 8 years ago

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.

valeriangalliat commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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

pascalduez commented 8 years ago

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 commented 8 years ago

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

carljm commented 8 years ago

See https://github.com/SassDoc/scss-comment-parser/pull/23 and https://github.com/SassDoc/sassdoc/pull/442

pascalduez commented 8 years ago

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

carljm commented 8 years ago

:+1: thanks for the review and merge!