Skip to content

fix(deps): Upgrade puppeteer using cache utils #540

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

Merged
merged 3 commits into from
Jan 24, 2023

Conversation

aitchiss
Copy link
Contributor

@aitchiss aitchiss commented Jan 23, 2023

Closes https://github.com/netlify/netlify-plugin-lighthouse/issues/510

Upgrades puppeteer, working around the new caching behaviour by making sure the puppeteer cache is saved/restored on each build.

Internal test steps:

  • Check running the plugin locally continues to work as expected across multiple test runs
  • This test site is using this branch's version of the plugin - retrying deploys (without any cache clearing) should result in lighthouse scores being generated consistently

@netlify
Copy link

netlify bot commented Jan 23, 2023

👷 Deploy Preview for plugin-lighthouse processing.

Name Link
🔨 Latest commit f7c6051
🔍 Latest deploy log https://app.netlify.com/sites/plugin-lighthouse/deploys/63cfb2ae67e6220009cef975

@aitchiss aitchiss requested review from jackbrewer and a team and removed request for a team January 23, 2023 16:28
@aitchiss aitchiss marked this pull request as ready for review January 23, 2023 16:28
@aitchiss aitchiss requested review from a team and removed request for a team January 23, 2023 16:28
@aitchiss aitchiss changed the title Upgrade puppeteer using cache utils fix(deps): Upgrade puppeteer using cache utils Jan 23, 2023
@aitchiss aitchiss self-assigned this Jan 23, 2023

export const onPreBuild = async ({ utils } = {}) => {
// Puppeteer relies on a global cache since v19.x, which otherwise would not be persisted in Netlify builds
await utils?.cache.restore(puppeteerCacheDir);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing is logged out during this step by default - I'm not sure if we want to add a simple console log here/when we save the cache, just for info... what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think it would be good to add something here and when saving. The Deploy Log now adds a section for onPreBuild, but without any content it may prompt questions.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah agreed, looks a bit weird empty like that. I've added a couple of lines:

jackbrewer
jackbrewer previously approved these changes Jan 24, 2023
Copy link
Contributor

@jackbrewer jackbrewer left a comment

Choose a reason for hiding this comment

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

Woo! Working great 💯

I think a couple more log lines could be nice, but happy either way


export const onPreBuild = async ({ utils } = {}) => {
// Puppeteer relies on a global cache since v19.x, which otherwise would not be persisted in Netlify builds
await utils?.cache.restore(puppeteerCacheDir);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think it would be good to add something here and when saving. The Deploy Log now adds a section for onPreBuild, but without any content it may prompt questions.

image

Copy link
Contributor

@jackbrewer jackbrewer left a comment

Choose a reason for hiding this comment

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

Yay! messaging looks good 🚀

@aitchiss aitchiss merged commit f831eb1 into main Jan 24, 2023
@aitchiss aitchiss deleted the sa/510/upgradePuppeteerUsingCacheUtils branch January 24, 2023 10:32
jackbrewer added a commit that referenced this pull request Jan 25, 2023
jackbrewer added a commit that referenced this pull request Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade puppeteer to v19, and refactor away from deprecated createBrowserFetcher
2 participants