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

Upgrade dependencies #246

Merged
merged 7 commits into from
Jul 30, 2021
Merged

Upgrade dependencies #246

merged 7 commits into from
Jul 30, 2021

Conversation

Clarity-89
Copy link
Contributor

@Clarity-89 Clarity-89 commented Jun 8, 2021

  • Upgrade puppeteer, typescript and prettier to the latest versions

Tested on Mac and it builds and works ok. Could use more tests on other OS probably.

Also did some brief profiling and it seems like total rendering time per panel is decreased by ~400ms, could be due to the newer versions of Chronium used.

@Clarity-89 Clarity-89 self-assigned this Jun 8, 2021
if (!config.rendering.chromeBin && (process as any).pkg) {
//@ts-ignore
Copy link
Contributor Author

@Clarity-89 Clarity-89 Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an issue with types, executablePath works but TS complains that it doesn't exist.

@Clarity-89 Clarity-89 added the dependencies Pull requests that update a dependency file label Jun 9, 2021
@Clarity-89 Clarity-89 marked this pull request as ready for review June 9, 2021 13:26
@Clarity-89 Clarity-89 requested a review from AgnesToulet June 9, 2021 13:26
@AgnesToulet
Copy link
Contributor

Did some tests on Windows and it crashes when running in plugin mode because of this issue: puppeteer/puppeteer#6563.
The fix is to close all the pages before closing the browser in the getBrowserVersion function:

  async getBrowserVersion(): Promise<string> {
    let browser;

    try {
      const launcherOptions = this.getLauncherOptions({});
      browser = await puppeteer.launch(launcherOptions);
      return await browser.version();
    } finally {
      if (browser) {
        const pages = await browser.pages();
        await Promise.all(pages.map(page =>page.close()));
        await browser.close();
      }
    }
  }

I need to do more tests with the different options of the plugin but it looks good otherwise!

@Clarity-89
Copy link
Contributor Author

@AgnesToulet are you able to fix the Windows issue? It's a bit hard for me to do as I don't have Windows machine setup.

Copy link
Contributor

@DanCech DanCech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AgnesToulet AgnesToulet merged commit d744c82 into master Jul 30, 2021
@AgnesToulet AgnesToulet deleted the upgrade-puppeteer branch July 30, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants