Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streams #5

Merged
merged 25 commits into from
Nov 14, 2015
Merged

Streams #5

merged 25 commits into from
Nov 14, 2015

Conversation

IonicaBizau
Copy link
Owner

I'm trying to add the stream functionality in the toPdf method. The goal is to be able to work with streams instead of files, when it's needed. Rendering in files is still supported.

Currently the toPdf function doesn't work because:

  • I don't know how to set custom html that comes not from a file but from a string value.
  • I don't know how to pipe the result pdf into a stream, not into a file.

However, the toHtml is working well. If no output field is specified in options, then the callback result will be the html string:

require("http").createServer(function (req, res) {
    myInvoice.toHtml(function (err, data) {
        res.end(data);
    });
}).listen(8000);

This will fix #4. 🔥

@IonicaBizau IonicaBizau mentioned this pull request Aug 7, 2015
@IonicaBizau
Copy link
Owner Author

Maybe node-html-pdf by @marcbachmann could help here! I'm gonna give a try. 😄

@stellanhaglund
Copy link

Looks promising! :)
Surprised by the fast responses and interest in the submitted issue! :)
Never happened to me before, really appreciate it!

fredag 7 augusti 2015 skrev Ionică Bizău notifications@github.com:

Maybe node-html-pdf https://github.com/marcbachmann/node-html-pdf by
@marcbachmann https://github.com/marcbachmann could help here! I'm
gonna give a try. [image: 😄]


Reply to this email directly or view it on GitHub
#5 (comment).

@IonicaBizau
Copy link
Owner Author

@idlyapp Hehe, I'm always trying to do my best. I'm really happy when people appreciate the work I've done. Donations are always welcome! Thanks! 🍀

@IonicaBizau
Copy link
Owner Author

@idlyapp I pushed a fix for the toPdf function. Please update your code (see the examples in test/index.js) and see what happens. Some tmp files are still created in /tmp dir. Hopefully, Heroku accepts that.

I opened an issue about that here: marcbachmann/node-html-pdf#61

@IonicaBizau
Copy link
Owner Author

Something like this:

require("http").createServer(function (req, res) {
    myInvoice.toPdf({ output: res });
}).listen(8000);

@stellanhaglund
Copy link

Sounds great will get back to you in a couple of h! Thx!

fredag 7 augusti 2015 skrev Ionică Bizău notifications@github.com:

@idlyapp https://github.com/idlyapp I pushed a fix for the toPdf
function. Please update your code (see the examples in test/index.js) and
see what happens. Some tmp files are still created in /tmp dir.
Hopefully, Heroku accepts that.

I opened an issue about that here: marcbachmann/node-html-pdf#61
marcbachmann/node-html-pdf#61


Reply to this email directly or view it on GitHub
#5 (comment).

@stellanhaglund
Copy link

Is res the file i should pass to s3?

fredag 7 augusti 2015 skrev Stellan Haglund sh@idly.se:

Sounds great will get back to you in a couple of h! Thx!

fredag 7 augusti 2015 skrev Ionică Bizău <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>:

@idlyapp https://github.com/idlyapp I pushed a fix for the toPdf
function. Please update your code (see the examples in test/index.js)
and see what happens. Some tmp files are still created in /tmp dir.
Hopefully, Heroku accepts that.

I opened an issue about that here: marcbachmann/node-html-pdf#61
marcbachmann/node-html-pdf#61


Reply to this email directly or view it on GitHub
#5 (comment).

@stellanhaglund
Copy link

No sorry that was http response.

As i understand it the variable i put after output: will be the file?

fredag 7 augusti 2015 skrev Stellan Haglund sh@idly.se:

Is res the file i should pass to s3?

fredag 7 augusti 2015 skrev Stellan Haglund <sh@idly.se
javascript:_e(%7B%7D,'cvml','sh@idly.se');>:

Sounds great will get back to you in a couple of h! Thx!

fredag 7 augusti 2015 skrev Ionică Bizău notifications@github.com:

@idlyapp https://github.com/idlyapp I pushed a fix for the toPdf
function. Please update your code (see the examples in test/index.js)
and see what happens. Some tmp files are still created in /tmp dir.
Hopefully, Heroku accepts that.

I opened an issue about that here: marcbachmann/node-html-pdf#61
marcbachmann/node-html-pdf#61


Reply to this email directly or view it on GitHub
#5 (comment).

@IonicaBizau
Copy link
Owner Author

No, it's the response stream which comes from the createServer callback.

@stellanhaglund
Copy link

Great news! now it's showing in browser! :)

But how would I send the file to for example s3?

@stellanhaglund
Copy link

The invoice rows doesn't seem to be rendered though..

@stellanhaglund
Copy link

I figured it out,

myInvoice.toPdf({
output: res
}, function (err, data){
console.log(err, data.path);
});

@IonicaBizau
Copy link
Owner Author

If you pass a stream to in the output field, then the pdf stream will be piped there. That means you can pass the request stream:

myInvoice.toPdf({ 
    output: Request("http://domain.com/upload-invoice") 
}, function (err, data){
    console.log(err, data.path);
});

It should work I guess... maybe with some changes. 😄

@stellanhaglund
Copy link

any idea why task rows isn't rendered?

@IonicaBizau
Copy link
Owner Author

@idlyapp Do you have an example (online)?

@stellanhaglund
Copy link

I actually did it a little different way, using express, so I simply do a post call to the url and the pdf is rendered and uploaded to where I want it.

This way I can simply put the invoice data as the body of the post, so I don't have a url where you will see the pdf just by loading it.

Is it working for you?

@IonicaBizau
Copy link
Owner Author

@idlyapp Thanks for the heads-up! I pushed a fix regarding the description rows.

@stellanhaglund
Copy link

Awesome! :)

@IonicaBizau IonicaBizau merged commit 0d99355 into master Nov 14, 2015
@IonicaBizau IonicaBizau deleted the streams branch November 14, 2015 19:55
@IonicaBizau
Copy link
Owner Author

🚢 2.0.0 🎉

@stellanhaglund Sorry it took so long to get this fixed, but finally it's happening! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do something with pdf
2 participants