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

Support an instance mode for node-html-to-image #95

Closed
wants to merge 1 commit into from

Conversation

joshk0
Copy link

@joshk0 joshk0 commented Jan 28, 2021

This is a breaking API change (for now)

node-html-to-image's default export is now a function which returns an
instance object with a render() method which is the old
nodeHtmlToImage() functionality. The reason for doing this is to allow
us to instantiate a single 'image maker' object that we call render() on
multiple times and reuse the same puppeteer cluster.

In addition,

  • Update the type definitions.
  • Use Cluster.execute() instead of Cluster.queue() plus Cluster.idle()
    as this will allow better use in concurrent situations. We will only
    wait for our jobs to finish rather than for the entire cluster to go
    idle.

Fixes #80

**This is a breaking API change (for now)**

node-html-to-image's default export is now a function which returns an
instance object with a render() method which is the old
nodeHtmlToImage() functionality. The reason for doing this is to allow
us to instantiate a single 'image maker' object that we call render() on
multiple times and reuse the same puppeteer cluster.

In addition,
* Update the type definitions.
* Use Cluster.execute() instead of Cluster.queue() plus Cluster.idle()
  as this will allow better use in concurrent situations. We will only
  wait for our jobs to finish rather than for the entire cluster to go
  idle.

Fixes frinyvonnick#80
@frinyvonnick
Copy link
Owner

frinyvonnick commented Feb 4, 2021

@joshk0 thank you for opening this PR 🙏

I answered you here.

Please make sure your code pass the tests 😄

@michael-eckhart-woellkart

Hi, we really need this feature. It would be great if that PR can be merged.

@frinyvonnick
Copy link
Owner

Hi @michael-eckhart-woellkart this PR does not work. Feel free to read the associated issue and propose a PR that fixes it 👌

@frinyvonnick
Copy link
Owner

I close this PR because I did not get any answer.

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.

Is it possible to reuse the puppeteer instance for better performance?
3 participants