Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update result node version and npm dependencies #140

Merged
merged 3 commits into from Aug 27, 2019

Conversation

BretFisher
Copy link
Member

The result Node.js app was years out of date, and also had known vulnerabilities. This PR has a list of improvements and fixes that I've been using in my own version for months and in my Docker for Node.js course. I tried to make minimal changes with these goals:

  • Use the current LTS release of Node.js
  • Update dependency packages to their latest releases (js refactor required)
  • Improve Dockerfile readability and more closely follow best practices

Changes include:

  • Use latest angular 1.4
  • To match other .js loading patterns in this repo, don't use a CDN angular, but rather a local copy in case examples are used offline or in large classrooms with slow internet.
  • The result app was years out of date with node and npm packages, and required multiple changes to use the currently supported socket.io and postgres libs.
  • Minimal changes to server.js to support change in postgres driver syntax.
  • A few minor changes to Dockerfile to improve readability and conform to docker best practices.
  • Change Dockerfile to use npm ci to ensure future builds don't break due to the difference between package.json and package-lock.json. ci only uses package-lock.json so builds will use the versions we know to work.
  • Change Dockerfile COPY to include package-lock.json in docker build.

Note: A separate PR #139 will prevent node_modules from being included in git commits and docker builds.

To match other .js loading patterns in this repo, don't use a CDN angular, but rather a local copy in case examples are used offline or in large classrooms with slow internet. Also updated to latest angular 1.4
The result app was years out of date with node and npm packages, and required multiple changes to use the current supported socket.io and postgres db libs. Minimal changes to server.js to support change in pg syntax. A few minor changes to Dockerfile to improve readability, conform to docker best practices, and change to use npm ci to ensure future builds don't break again due to difference between package and package-lock.
@BretFisher
Copy link
Member Author

Changed from node:10-alpine to node:10-slim to work around a npm+alpine bug that is preventing Docker's Jenkins PR builder from completing. Other PR's are failing too, but I can rebase them once this PR is merged.

@jrburcio jrburcio merged commit bfa3d14 into dockersamples:master Aug 27, 2019
@BretFisher BretFisher deleted the update-result-deps branch May 3, 2021 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants