PejmanNik/puppeteer-report

Support for custom puppeteer/browser/page instances

Closed this issue · 7 comments

federicojasson commented

Hi, I found your repo mentioned in this issue: puppeteer/puppeteer#5345 (comment)

First of all, thank you for sharing this workaround.

I was wondering how hard do you think it would be to support custom puppeteer/browser/page instances. I'm using chrome-aws-lambda to generate PDFs on AWS Lambda. That library customizes the way the puppeteer and the browser instances are initialized:

    browser = await chromium.puppeteer.launch({
      args: chromium.args,
      defaultViewport: chromium.defaultViewport,
      executablePath: await chromium.executablePath,
      headless: chromium.headless,
      ignoreHTTPSErrors: true,
    });

That makes it impossible to use your library.

From looking at the code really quickly, it doesn't look like allowing customization would affect the approach in general. To make it backward-compatible, maybe a third object could be received in the pdf(...) function, including factory functions for puppeteer, browser and page (defaulting to the current approach):

pdf(..., ..., {
  puppeteer: () => { ... },
  browser: puppeteer => { ... },
  page: browser => { ... }
}

Customizing the page could allow to set very specific options like setDefaultNavigationTimeout(...).

BTW, I can lend a hand if you think this is a good idea.
Thanks!

PejmanNik commented

Hi, Thank you for using it. 😁

Thank you for sharing your idea. 👍 That is great.

We just need the Page, so I'm thinking of a public base method that gets Page is input and applies the process, then changes the current pdf method to create the page and pass it to that primary method. This way, we solve your issue and make it possible to use it over none static pages and make it fully controllable from outside.

I can work on it on Friday (our weekend :) ) and love to hear your feedback. Also, I'm entirely open to PR.

PejmanNik commented

Please try the v1.1.0 and the new pdfPage method.

federicojasson commented

@PejmanNik This is great! Thank you for adding this feature, and sorry for the late response.

I've been testing it a little bit and works locally. There are still a couple of issues when it comes to AWS Lambda (I didn't foresee this). Since the library has a hard dependency on puppeteer, the package size is too large (Lambda has a size limit on the package size). When using chrome-aws-lambda in a Lambda environment, only puppeteer-core is really needed (lightweight), since a pre-compiled binary is included already with the chrome-aws-lambda library. More details here: https://github.com/alixaxel/chrome-aws-lambda#install

My guess is that it should be possible to add puppeteer as an peer dependency and puppeteer-core as a regular dependency (PDFOptions and Page are both part of puppeteer-core). For pdf, puppeteer would be required, but not for pdfPage. In that case, the puppeteer import statement would need to happen inside the pdf function.

federicojasson commented

I've created a PR to try out that alternative: #3

  • 👍1
PejmanNik commented

Thank you for your PR 👍
I think there is still room to decrease the size of the package. I can remove the bundle.js file from the npm package so I think, we have a release tonight including your PR and some minor changes

PejmanNik commented

Thank you @federicojasson. we have a new release for your great contribution

https://github.com/PejmanNik/puppeteer-report/releases/tag/v2.0.0

  • 🎉1
federicojasson commented

@PejmanNik Yay! I can confirm I was able to deploy a working application into AWS Lambda. Thank you too for working on these changes.

  • ❤️1