Allowing onBeforePrint to return a Promise

[aviklai]

#140

[MatthewHerbst]

Thanks for jumping on this! Just a few minor things and then I think we'll be good to go (we'll have to release this under a breaking change but I think that's ok)

[aviklai]

@MatthewHerbst Why is this a breaking change? we still allow the onBeforePrint function to return void.

[MatthewHerbst]

This line:

onBeforePrintOutput instanceof Promise

will break if Promise does not exist. Specifically, if you run that on a browser (I tested on IE10 via Browserstack) without Promise, you will get the error:

var x = {}
x instanceof Promise
// Error: "'Promise' is undefined"

So the implementation as it would still require users to polyfill Promise for environments it doesn't exist in, which is not currently a library requirement, which is why it would require a breaking change.

Maybe there is another way to check if something is a Promise?

[aviklai]

Apparently, there is no standardized way:
https://stackoverflow.com/questions/27746304/how-do-i-tell-if-an-object-is-a-promise

This is the es5 answer that we can use (and graphql-js use):
https://github.com/graphql/graphql-js/blob/master/src/jsutils/isPromise.js

So it would look like this:
if (onBeforePrintOutput && typeof onBeforePrintOutput.then === 'function')

What do you think?

[MatthewHerbst]

I'm ok with that. It's not 100% perfect (as some commenters in the accepted answer of that SO post point out) but I think it's good enough

[aviklai]

@MatthewHerbst Done - changed the way to check if it's a promise.

[MatthewHerbst]

Awesome! One last request: can you please squash the commits together?

[aviklai]

@MatthewHerbst Done.

[MatthewHerbst]

Awesome, thanks! I'll get a new version with this published in a little bit

[MatthewHerbst]

Published as v2.2.0. Thanks again!