Omit dev deps when installing Auspice #165
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 was referenced Jun 22, 2023
|
I think this is getting skunked by the old version of npm (v6) in the image, which I only noted because I saw this warning float by: |
#122, hopefully to be merged soon, will bring a newer version of npm. |
|
Confirmed that with newer npm in the container this works fine. commit d38eb85c003fb738cec4a7df7aabc768ec610149
Author: Thomas Sibley <tsibley@fredhutch.org>
Date: Fri Jun 23 10:11:25 2023 -0700
wip! upgrade npm
diff --git a/Dockerfile b/Dockerfile
index 569bfd7..e248fdf 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -35,7 +35,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
# Install a specific Node.js version
# https://github.com/nodesource/distributions/blob/0d81da75/README.md#installation-instructions
RUN curl -fsSL https://deb.nodesource.com/setup_14.x | bash - \
- && apt-get update && apt-get install -y nodejs
+ && apt-get update && apt-get install -y nodejs && npm install -g npm@8
# Used for platform-specific instructions
ARG TARGETPLATFORMWill test some more locally, clean up, and then mark for review. |
tsibley
force-pushed
the
trs/omit-auspice-dev-deps
branch
from
69e468c
to
693d639
Compare
tsibley
force-pushed
the
trs/existing-registry-container
branch
from
75e2680
to
bbda96b
Compare
This was introduced incidentally, and seemingly unintentionally¹, in "Replace Alpine-based image with a Debian-based image" (ce07dad), but no one noticed. `npm update` is unwarranted because it's intended for maintainers of a package, not downstream user installs, which is the role we have here. Its effect was to bump the minimum versions in Auspice's package.json to the latest available (while still respecting SemVer constraints) and then install all of Auspice's deps into node_modules/.² That's unnecessary because we then run `npm install`, which unlike `npm update`, also runs pre/post-installation steps Auspice includes. ¹ <#21 (comment)> ² Notably, `npm update` was added when the image had npm v5, which updates both package.json and package-lock.json. We currently have npm v6, which does the same. Subsequent versions, e.g. npm v8, stopped updating package.json and only update package-lock.json.
This avoids disk-heavy deps like Puppeteer, which bundle a whole Chromium install. Based on some rough comparisons, I expected this to shave about 600MB from the uncompressed image size and a local test build bore that out. The node_modules/ tree is infamously large and bloated. The usual culprits are non-source files that are commonly included in package distributions but not needed at run time. So there is surely more we could shave off here, but this is a huge easy start.
tsibley
force-pushed
the
trs/omit-auspice-dev-deps
branch
from
693d639
to
d5d1662
Compare
|
Dropped the |
victorlin
approved these changes
Jun 23, 2023
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 avoids disk-heavy deps like Puppeteer, which bundle a whole Chromium install. Based on some rough comparisons, I expected this to shave about 600MB from the uncompressed image size and a local test build bore that out.
The node_modules/ tree is infamously large and bloated. The usual culprits are non-source files that are commonly included in package distributions but not needed at run time. So there is surely more we could shave off here, but this is a huge easy start.
Testing