More Node like API

[sindresorhus]

The API is currently a bit awkward as it doesn't follow the Node convention.

Instead of:

sass.render({success: function () {}, error: function () {});

It should be:

sass.render({}, function (error, result) {});

Or embrace promises:

sass.render({}).then(function() {}).catch(function () {});

[am11]

We refined API in v2 (#547), but I agree this is not the end. Promises is the best approach IMO and the syntax will be compatible with PostCSS as well.

Incidentally, with:

sass.render({}).then(function() {}).catch(function () {});

in render() are you proposing to send in options object and then() will have a success callback?

We would also need to consider custom importers and the upcoming custom functions (#644).

[sindresorhus]

Promises is the best approach IMO

👍 I would go for a ES6 Promises polyfill: https://github.com/jakearchibald/es6-promise

in render() are you proposing to send in options object and then() will have a success callback?

Yes.

We would also need to consider custom importers and the upcoming custom functions (#644).

They belong in the options object.

[am11]

@sindresorhus, what do you think if we instead go with reactive pattern:
http://reactive-extensions.github.io/RxJS/?

It claims to render even promise pattern; traditional.

[sindresorhus]

I like the reactive pattern, but I strongly feel the API should be promise based as it's the standardized describe async operations. Users can easily use any reactive pattern upon a promise interface.

[dlmanning]

My two cents: please not promises. Error handling from promises is still kind of fubar. It's trivially easy to wrap the traditional node callback pattern in a promise if your project is using them, but I think it's still premature for a project as widely depended upon as node-sass to adopt them for its api.

[xzyfer]

My thoughts here. Leave the existing callback API. I'm aware it's not popular opinion, but it is most accessible.

As @dlmanning points out it's trivial to progressively enhance callbacks to promises, the reverse not so much (it's certainly doable). Arguing that anyone who uses node-sass should be using promises because it's the right way is no argument at all.

As for the reactive approach, all my points on promises apply, with the addition that it has yet to see wide adoption. Add to that native async/await is coming in ES7, making this the primary API seems premature.

Generally speaking I believe we should aim for providing the lowest developer friction to widest audience, providing sane defaults were possible. If we're going to jump through hoops to support old CentOS I see no reason not to give the attention to the node side of things.

[sindresorhus]

I'm fine either either callback or promise.

[am11]

I agree with the notion: if it is not broken do not fix it.

But just for sake of argument, what is at risk or what kind of learning curve we are anticipating, if node-sass require the .then() syntax:

sass.render({
  data: 'a{b: c}'
}).then(function(result) {
    // process result
});

instead of:

sass.render({
  data: 'a{b: c}',
  success: function(result) {
    // process result
  }
});

[sindresorhus]

Why did you close it? The API is still awkward and not normal Node style callback API...

[xzyfer]

I agree with @sindresorhus at the very least the callback signature should follow the node convention of

function (error, result) {}

[dlmanning]

node callback convention FTW

[am11]

@xzyfer, earlier you were suggesting "Leave the existing callback API" and that there is not point of refactoring..

[xzyfer]

Yes. I'm saying leave the callback API. However in the node callback convention is callback(err, ..rest). I was under the impression @sindresorhus was suggesting the argument order was reversed in node-sass (I haven't looked into this myself).

If that is the case, it should be addressed.

[kevva]

node-sass uses an object as callback as opposed to the usual node style (err, res).

sass.render({
  success: function () {},
  error: function () {}
});

This is pretty confusing imo.

[am11]

@kevva, yes that looks confusing. What are your thoughts on promise pattern?

[am11]

This was addressed by 69a4560 via #684, in a backward compatible manner.

[am11]

Update to this. We are skipping v2.1 and planning to release v3.0 as the next release instead, so to introduce various breaking changes.

For that matter, we are dropping the old method cb style in favour of node-style cb.

Credit goes to @Jakobo for #726.