generate stream without promise

[graingert]

streams are already async, and don't need to be wrapped in a promise.

[graingert]

This is just a WIP as I'm not sure what the interface should be.

[graingert]

I think await generate(...).then(v => v.toStream()) should be deprecated.

[westy92]

htmlPdf.create(html, options) is what needs to be wrapped in a Promise. If you look here, the Stream is not being wrapped in a Promise.

Here's example usage:

const pdf = await htmlPdf.create(html, options);
const stream = pdf.toStream(); // Look mom, no Promises!

The only async CreateResult is:

async toFile(filename: string): Promise<void>

[graingert]

@westy92 code should be:

const stream = htmlPdf.createToStream(html, options); // Look parent, no Promises!

[graingert]

@westy92 because const pdf = await htmlPdf.create(html, options); is promise code that's not needed if the user wants a stream.

[codecov]

Codecov Report

Merging #43 into master will decrease coverage by 6.77%.
The diff coverage is 20%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage     100%   93.22%   -6.78%     
==========================================
  Files           3        3              
  Lines         108      118      +10     
  Branches       12       12              
==========================================
+ Hits          108      110       +2     
- Misses          0        8       +8

Impacted Files	Coverage Δ
src/index.ts	86.88% <20%> (-13.12%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a4f21c...b202f82. Read the comment docs.

[graingert]

@westy92 obviously I'll update the tests if you agree with this interface.

[westy92]

I'm not sure what's with the Promisephobia. The reason results are extracted into a CreateResult is so it can be abstracted away and then consumed in a desired format. Adding a special case just for Streams seems counterintuitive.

Can you provide a use case for needing this functionality?

[graingert]

@westy92 I don't have Promise phobia, I just don't want them in the stream interface where they're unused.

[graingert]

@westy92 eg it would match the node API for createReadStream: https://nodejs.org/api/fs.html#fs_fs_createreadstream_path_options

[westy92]

Your implementation is just hiding the Promise; it's still there. If you want to do this, feel free to put this wrapper inside your own project.

[graingert]

@westy92 sure it's hiding the promise, that's what streams are for.

[graingert]

@westy92 if you were to add an Observable interface, eg when they land natively in node, it would look very strange to do:

Observable.from(htmlPdf.create(html, options)).flatMap(v => v.toObservable());

vs:

htmlPdf.createObservable(html, options);

Which is analogous to the current stream API.

[westy92]

It appears that https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-printToPDF now has a transferMode option:

That should allow us to implement this fairly simply.