Resolve toBuffer Promise with an array (was: When using promises, how do you get image info)

[salzhrani]

if a callback is used with toBuffer for example the image info is passed to the callback, how do you get this info when using a promise ?

[lovell]

As you've probably discovered, the single value passed to resolve a Promise is the Buffer containing image data only, which does not include the info Object. IIRC this was to prevent the API breaking - Promises were supported before the info Object was added.

I believe the ES6 Promises spec will provide spread as well as then. If so, this library could start to resolve a Promise with an array containing [data, info] (which the consumer could "spread" over multiple parameters), but that would still involve a breaking change to the API.

It's not entirely elegant, but with the code as it is right now you should be able to chain a call to resize with a call to metadata to achieve what you want using only Promises:

sharp(input).resize(123).toBuffer()
  .then(function(data) {
    return [data, sharp(data).metadata()];
  })
  .spread(function(data, info) {
    // now have data and info
  });

[salzhrani]

Is it possible to maybe pass a boolean to toBuffer() making it return an array instead of a buffer? would this break the API ?

[lovell]

I'm not a big fan of boolean parameters as they encourage the use of inline true or false values that make understanding/maintaining client code difficult. For example, the difference in behaviour between toBuffer(true) and toBuffer(false) is not immediately obvious, at least not to me when revisiting my own code after six months weeks 😃.

A named parameter in the form of an Object would be better, but given array-returning Promises are soon to be a "standard" thing, I think I'd prefer to break the API (as part of a major version revision) to always return an array.

If it's OK with you I'd like to change this issue into a feature request to have the Promise resolve with a [data, info] array instead of data only.

Were you able to try the workaround with the chained called to metadata?

[salzhrani]

I am using a workaround so things are currently ok. please edit and/or close this issue as you see fit. Thanks

[lovell]

Cheers Samy, thank you for reporting this inconsistency in the API in the first place, much appreciated.

[strarsis]

Workaround using the Bluebird promise library:

var sharp   = require('sharp'),
    Promise = require('bluebird');

Promise.promisifyAll(sharp.prototype, {multiArgs: true});
  // promisify the sharp prototype (methods) to promisify the alternative (for raw) callback-accepting toBuffer(...) method

// for demonstration purposes
sharp('./test.png')
.raw().toBufferAsync() // notice the [...]Async method name, generated by bluebird
.then(function(dataAndInfo) {
    // `multiArgs: true` further above will spread the arguments passed to the callback function to array elements by their order
  var data = dataAndInfo[0], // first the data, then the info
      info = dataAndInfo[1];
        // nicely assigning the array elements back to variables for demonstration purposes

  return sharp(data, {raw: info}) // as usual...
  .metadata(); // triggering sharp to actually load that data (for demonstration purposes that this works)
})
.then(function(meta) {
  console.log(meta); // should succesfully log the determined metadata
});

Hope this helps!

[lovell]

Given there's no easy deprecation route how about we go with a slight modification of @salzhrani's original suggestion of allowing toBuffer to accept a parameter indicating the desired behaviour?

This would look something like toBuffer({ resolveWithArray: true }), defaulting to false, the current behaviour.

[Vispercept]

From my point of view, it would be better if there would be a completely new function called getInfo for example. This function should return all info. toBuffer returning also info is a nice side-effect and one should not rely on that. Splitting the functionality oft toBuffer into two functions would also follow the clean-code principles (Functions should only do one thing – https://github.com/ryanmcdermott/clean-code-javascript/blob/master/README.md#functions-should-do-one-thing)

[lovell]

@Vispercept The data and info attributes "returned" by toBuffer are highly related to each other; info contains properties computed whilst processing the final output image. Perhaps you're thinking of the existing metadata call to access input image info?

Rather than the Promise resolving with an Array, how about resolving with an Object, which would allow for destructuring e.g.:

sharp(input)
  .resize(123)
  .toBuffer({ resolveWithObject: true })
  .then( ({ data, info }) => {
    ...
  });

[Vispercept]

@lovell thanks for the clarification :-) didn't know that...

[lovell]

Commit 6fe5b30 adds the new resolveWithObject option to toBuffer. This will be in v0.17.3.

[lovell]

v0.17.3 now available, thanks everyone for the suggestions and discussions.