Bugfix: convert variable to string if buffer

[sambonbonne]

Hello,

I found a little "random" bug in a package that use css as a dependency : sometimes the css variable in lib/parse/index.js is a buffer.
I actually use Node 4.2.1, this bug appeared since I upgraded from 0.12.7 to 4.1 (so I imagine that Node 4.0 broke something).

The module which use your package seems to work since I applied this fix, so if you include this 6 lines it would be great.

Thanks in advance!
Best regards,
Samuel D.

PS : sorry for my english (I imagine you hear/read this a lot)

[conradz]

You should be able to fix this in your code that uses this module. Example:

var fs = require('fs')

// Instead of:
css.parse(fs.readFileSync('test.css'))
// Use this:
css.parse(fs.readFileSync('test.css', 'utf8'))

// Or, instead of:
var contents = fs.readFileSync('test.css')
css.parse(contents)
// Use this:
var contents = fs.readFileSync('test.css')
contents = contents.toString()
css.parse(contents)

// If you use rework, instead of:
rework(fs.readFileSync('test.css'))
  // ...
// Use this:
rework(fs.readFileSync('test.css', 'utf8'))
  // ...

The point is you should be able to convert the input into a string before passing it to css.parse. The documentation for css.parse says it takes a "CSS string", so passing a buffer is not supported. You should find what is causing the buffer to be passed in and convert it to a string before passing it to css.parse.

[sambonbonne]

I understand your point of view, but I thought allowing Buffer in your package directly could make the Node update smoother for other packages which depend on yours.
Are you sure you can't consider using my fix ?

I can agree that you want your module to stay KISS, if you really don't want to use my patch I'll try to fix the package I use which depends on yours.

[conradz]

Yeah, if the Node update broke a package I would think that would indicate a bug in that package if it sometimes uses Buffer and sometimes doesn't. The parse method takes a string and shouldn't need to handle all other types of input. A better option would be to add an informative error message when a Buffer is passed to the method, so you could quickly tell that the problem is that you passed a Buffer when it is expecting a string.

[sambonbonne]

I made a pull request to the concerned package, I hope they will accept it.

Concerned css, maybe it would be useful to make a parameter check, like :

if (typeof css !== 'string') {
    throw new TypeError('Expected String, received ' + (typeof css));
}

But if this verification is done for one function, it should be done for all ...
Correct me if you think I'm wrong, but make this check just for one variable in one function isn't coherent.

If you decide to put verifications on many functions, feel free to ask for help :)