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 shrinkwrap #237

Merged
merged 1 commit into from Nov 19, 2021
Merged

Upgrade shrinkwrap #237

merged 1 commit into from Nov 19, 2021

Conversation

hgwood
Copy link
Contributor

@hgwood hgwood commented Nov 19, 2021

Hello, and thank you for making decktape!

This PR upgrades npm-shrinkwrap.json to lockfile version 2, which is the format introduced by npm v7. This fixes chrome not being downloaded when installing decktape as a dependency in a project using npm v7+. See puppeteer/puppeteer#6586 (comment). The new format is backward compatible so users of npm v6 and lower should not be impacted.

This upgrades npm-shrinkwrap.json to lockfile version 2, which is the format introduced by npm v7. This fixes chrome not being downloaded when installing decktape as a dependency in a project using npm v7+. See puppeteer/puppeteer#6586 (comment). The new format is backward compatible so users of npm v6 and lower should not be impacted.
hgwood added a commit to Zenika/sensei that referenced this pull request Nov 19, 2021
This simplifies setup and avoids a recuring issue when running "npm install" in the main package would result in infinite looping of calling the postinstall script. It also makes sensei publisable to npm more easily.

Due to puppeteer/puppeteer#6586 (comment), chrome was not being downloaded when running npm install. I had to fork and patch decktape. Let's hope the patch is merged and released. See astefanutti/decktape#237. Otherwise we could publish the fork.
Copy link
Owner

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

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

Thanks a lot. I'll cut a release ASAP.

@astefanutti astefanutti merged commit 1fdb3c2 into astefanutti:master Nov 19, 2021
hgwood added a commit to Zenika/sensei that referenced this pull request Nov 22, 2021
This simplifies setup and avoids a recuring issue when running "npm install" in the main package would result in infinite looping of calling the postinstall script. It also makes sensei publishable to npm more easily.

The PDF part was originally split for the main package to avoid having to install Chrome twice when developing, often in cases where I wasn't even touching the PDF part. However, as mentioned by @patou, a better technique to do that is to run `npm install` with `PUPPETEER_SKIP_CHROMIUM_DOWNLOAD` defined.

This also downgrades puppeteer from 11.0.0 to 10.4.0 to align to the version used by decktape 3.3.0 and allow npm to dedupe puppeteer and reduce the number of Chrome installs to 1. It does work in contrary from what I said in #120.

Due to puppeteer/puppeteer#6586 (comment), chrome was not being downloaded when running npm install. I had to fork and patch decktape. See astefanutti/decktape#237. The patch has been merged but not release so for now package.json points to the master branch of decktape instead of release. This should be changed as soon as decktape has a new release.
@astefanutti
Copy link
Owner

I've just released version 3.4.0.

@hgwood
Copy link
Contributor Author

hgwood commented Nov 28, 2021

Thank you!

@hgwood hgwood deleted the upgrade_shrinkwrap branch November 28, 2021 18:02
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.

None yet

2 participants