♻ merge pdf package in main package #121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
Build fails because the alpine-chrome base image that we use seems to not be able to handle the git npm dependency. This should be fixed soon though because the maintainer of decktape juste merged my PR 👍. |
patou
approved these changes
Nov 19, 2021
|
Still no release of decktape and I don't want to keep this PR up because it modifies package-lock.json and will conflict with other upgrades, so I merging it and will revisit when decktape has a new release. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 installwithPUPPETEER_SKIP_CHROMIUM_DOWNLOADdefined.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. Let's hope the patch is merged and released. See astefanutti/decktape#237. Otherwise we could publish the fork.