bump docker container to nodejs 22 and manually download phantomjs #6553
Conversation
…is truly not needed
…houldn't be installed anyway since its a devDependency, but let's handle that as a separate optimization
DougReeder
left a comment
DougReeder
left a comment
There was a problem hiding this comment.
Works fine in my instance.
…e (antipattern) of git dependencies pulling in their devDependencies with preinstall checks.
…cript shenanigans to execute phantomjs, and we want to try only upgrading one aspect at a time
…propriate architecture
matthargett
changed the title
Exairnous
left a comment
Exairnous
left a comment
There was a problem hiding this comment.
@matthargett LGTM. Thanks.
@ajlennon reported in Discord that it worked for him as well for both amd64 and arm64.
I noticed that this doesn't bump the node version mentioned in the readme, should it? Or is that something you're leaving for #6549
It does bump the node image version from 16.x to 22.x. Were you expecting a separate version bump in terms of an "engines" directive? |
|
I believe you added an "engines" directive in your other pull request for Node 22, but no, I was referring to the fact that the readme says "We use 16.16.0 on our build servers." and this isn't changed in this pull request to the new version of 22. |
|
The current version of this (f96fc6d) builds without issues on iMac with 8-Core Intel Core i7, and the image installs fine on my instance, and passes the smoke test for Firefox 139.0.1 (64-bit), Chrome Version 137.0.7151.104 (Official Build) (x86_64), Oculus Browser 39.0.0.15.... on the Quest 3 and Chrome 136.0.7103.125 on a Pixel 5 running Android 18. Still can't enter a room from the lobby on my iPhone, so that's no regression. |
|
Okay, since everyone is happy with the latest version of this PR, I'm merging it. The update to the Node version in the readme can be added in a subsequent PR. |
3 participants
What?
This tests upgrading the docker container to a newer Debian without phantomjs. PhantomJS appears to be a vestigial dependency that isn't used by anything in the repository, nor as a runtime
dependencyin any transitive sub-dependencies, but it is checked for by some ancient package in a manual lifecycle script.Why?
This is to enable the modernization of packages and infrastructure in #6549 more incrementally.
How to test
Docker image builds and deploys.
Documentation of functionality
Docker file is self-documenting, and this should be a seamless transition.
Alternatives considered
We tried to fully eliminate the need for phantomjs, but it's not possible without duplicating dependency upgrade efforts in #6549. Once #6549 is merged, we can retry relieving the phantomjs constraint (archived and abandoned for over 5 years now).
Additional details or related context
in support of #6549