Test with Node 20, updated deps + js-wacz

[matteocargnelutti]

How I tested these changes:

• docker compose up -d --build to force a rebuild
• In the web container's shell:
  • npm run build does generate valid (?) bundles
  • invoke run successfully runs npm run build
  • npx js-wacz --help prints js-wacz's help menu

Tests are passing and the app behaves normally when tested locally.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.22%. Comparing base (6b447bc) to head (efa78c0).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3486   +/-   ##
========================================
  Coverage    71.22%   71.22%           
========================================
  Files           48       48           
  Lines         6512     6512           
========================================
  Hits          4638     4638           
  Misses        1874     1874           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rebeccacremona]

This looks great!!!

I tested by:

• Removing my local node_modules folder, to make sure it didn't accidentally sync a cache of the old dependencies into the container
• Removing the perma_node_modules volume, for the same reason
• Checking out your branch
• Adding a test-node-dependencies suffix to the web image's tag to force a rebuild without clobbering the 105-28fef2ca2ecd1c74ba3059f0b59a4d34 image
• docker compose up, and watching the build
• docker compose exec web invoke run, with CELERY_TASK_ALWAYS_EAGER = False
• Doing a side-by-side comparison of the CSS on the Perma dashboard with prod
• Running through all of the javascripty parts of the app:
• • making a new folder
• • renaming an existing folder
• • moving a link between folders
• • viewing an old link batch
• • making a new link batch
• • switching folders using the folder tree
• • switching folders using the dropdown
• • expanding and collapsing a link's details in the link list
• • the "upload your own" flow
• • changing a link's options using the controls in the "tray" on the playback page
• • clicking through each tab of the /manage/stats page

I can't think of anything else to check!!

[jcushman]

I recommend pinning everything in docker, ideally to the version in prod -- anything you don't pin will eventually waste an hour when it breaks on CI instead of on your local, or breaks on prod instead of CI. If keeping the pin updated is wasting more time than that I could be talked out of it.

[bensteinberg]

The mechanism for installing node has changed; I believe it's no longer possible to pin to a specific .deb file exactly like we used to; the new mechanism is at nodesource/distributions#33 (comment)

[rebeccacremona]

ideally to the version in prod

I note that we do not run nodejs in prod for Perma: it only ever runs inside the docker container

Without pinning here, I think the biggest possible consequences are, a discrepancy between different developers' environments, and an accidental upgrade to a new minor release breaking something that was previously working. Neither of those feel like big risks to me.

[bensteinberg]

See also https://github.com/nodesource/distributions#new-update-%EF%B8%8F and https://github.com/nodesource/distributions/wiki/How-to-select-the-Node.js-version-to-install

[matteocargnelutti]

Yep, I think this is fair @jcushman - I personally feel okay using this Node.js 20.x installer unpinned since it Node 20 is already in LTS mode. That of course doesn't mean it's not going to receive an update that is going to break our setup, but makes it less likely 😅 ?

[matteocargnelutti]

@bensteinberg @jcushman I believe this would work, should I push and see if it actually does?

# node.js 
RUN curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && apt-get install -y nodejs=20.12.0-1nodesource1

[bensteinberg]

Perfect, I was about to make this exact suggestion, and point out that I just installed Debian updates for node on a few machines.

[rebeccacremona]

@matteocargnelutti is there a build relic to remove after the fact, like the rm move we had before? asking just because this image is so big

[matteocargnelutti]

@rebeccacremona As in: it got much bigger with that change? I am not sure 👀 !

[rebeccacremona]

@matteocargnelutti no, I just saw that there wasn't an rm or clean or whatever and thought maybe that was by accident. Ben just checked and it is no big deal, so never mind :-)

[jcushman]

This is awesome -- I think ship it if Ben is happy? As Becky points out we use pre-built assets in production, so the testing she did is likely to have found any problems that might exist.

[matteocargnelutti]

Thank you all for having a look and for the helpful feedback 👋