fix: Halt if npm install fails; switch to npm ci as well

[timmc-edx]

We've seen a few cases where npm package installation fails, but the server continues to try to start up, and then developers get very mysterious errors that are hard to debug. Fail fast on MFE startup to avoid this.

Also switch to npm ci so that:

1. Two developers on the same repo commit get the same version
2. Repository working directories are not modified by just running an MFE

(Also, fix a typo.)

See edx/edx-arch-experiments#335

---

I've completed each of the following or determined they are not applicable:

• Made a plan to communicate any major developer interface changes (or N/A)
  • Will check in a frontends Slack channel before merging, and announce in usual ways afterwards (following arch team's "How We Announce")

[timmc-edx]

Author's note: One uncertainty I have is whether there's a use-case where developers use the Docker container to run npm commands, including running npm install there in order to regenerate the package lock file. In that case, this fast-fail might not be great for everyone. Is that a concern, or do frontend developers basically always have npm installed globally?

[davidjoy]

My gut reaction says that if someone needs to re-generate or mess with their package-lock.json file, they can do that manually and not rely on it happening as a side-benefit of running a Docker command. It should be rare and done with intention. So long as folks have a recourse, then it seems a relatively safe change that won't be too disruptive.

[davidjoy]

It's worth noting that in edX's frontend build process, we still don't actually use npm ci so this is actually making devstack different than production. See this line: https://github.com/openedx/tubular/blob/master/tubular/scripts/frontend_utils.py#L59

And also note we apparently tried to change that and reverted it, according to what I see here: openedx-unsupported/tubular#670

Good times.

[davidjoy]

How often is failing fast a problem? And when that happens, how does someone know that it failed because of npm ci and see output so they can diagnose? I'd hate to do this and for people to have no idea why something went wrong. 🤔

[timmc-edx]

...huh. So I guess our lockfiles are just sort of... aspirational right now?

[timmc-edx]

I don't know what it currently looks like without the fail-fast, but here's what it looks like after the || exit 1 is added:

(venv-3.8) timmc@edx-timmc:~/edx-repos/devstack$ make frontend-app-learning-logs
docker-compose logs -f --tail=500 frontend-app-learning
Attaching to edx.devstack.frontend-app-learning
npm ERR! code EUSAGE
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! Missing: left-pad@1.2.0 from lock file
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! Clean install a project
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! Usage:
edx.devstack.frontend-app-learning | npm ERR! npm ci
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! Options:
edx.devstack.frontend-app-learning | npm ERR! [-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer|--save-bundle]
edx.devstack.frontend-app-learning | npm ERR! [-E|--save-exact] [-g|--global] [--global-style] [--legacy-bundling]
edx.devstack.frontend-app-learning | npm ERR! [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
edx.devstack.frontend-app-learning | npm ERR! [--strict-peer-deps] [--no-package-lock] [--foreground-scripts]
edx.devstack.frontend-app-learning | npm ERR! [--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run]
edx.devstack.frontend-app-learning | npm ERR! [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
edx.devstack.frontend-app-learning | npm ERR! [-ws|--workspaces] [--include-workspace-root] [--install-links]
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
edx.devstack.frontend-app-learning | npm ERR! 
edx.devstack.frontend-app-learning | npm ERR! Run "npm help ci" for more info
edx.devstack.frontend-app-learning | 
edx.devstack.frontend-app-learning | npm ERR! A complete log of this run can be found in:
edx.devstack.frontend-app-learning | npm ERR!     /root/.npm/_logs/2023-06-26T16_20_57_718Z-debug-0.log
edx.devstack.frontend-app-learning exited with code 1

I can also separate the fail-fast behavior to a separate PR if it would make more sense that way. (The fail-fast is actually what I primarily want to fix at the moment.)

[johnnagro]

I'm supportive of Tim's change - this is how we properly honor our lock files. If other parts of our build (or worse deploy) processes do not honor lock files then we should also address those ASAP because we should have high confidence as to what specificly we're building and deploying - right down to the specific versions of dependencies.

[davidjoy]

@johnnagro 's confidence gives me strength. 👍🏻