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

Use Node 20, and add it to CI #5012

Merged
merged 12 commits into from
Aug 30, 2024
57 changes: 44 additions & 13 deletions .github/workflows/npm-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,38 +8,69 @@ on:
jobs:
build:
runs-on: ubuntu-20.04

strategy:

matrix:
node-version:
- '16.15.0' # prior pinned Node version supported by kpi
- '20.17.0' # version pinned for kpi release
- '20' # latest available v20
- '22' # latest available v22
fail-fast: false # Let each job finish

steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
# We need this particular version, as npm 8.5.5 is the last version
# that works with our package.json :sadface:.
node-version: '16.15.0'
node-version: ${{ matrix.node-version }}
check-latest: true # download newer semver match if available
cache: 'npm'

- name: Identify resolved node version
id: resolved-node-version
run: echo "NODE_VERSION=$(node --version)" >> "$GITHUB_OUTPUT"
- name: Add "Node ${{ steps.resolved-node-version.outputs.NODE_VERSION }}" to summary
run: echo "${{ matrix.node-version }} → **${{ steps.resolved-node-version.outputs.NODE_VERSION }}**" >> "$GITHUB_STEP_SUMMARY"

# Set up Chrome, for the unit tests
- uses: browser-actions/setup-chrome@latest
- run: chrome --version
- name: Check for cached node_modules

# Cache node_modules, keyed on os, node version, package-lock, and patches
- uses: actions/cache@v4
name: Check for cached node_modules
id: cache-nodemodules
uses: actions/cache@v3
env:
cache-name: cache-node-modules
with:
path: node_modules
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json', 'patches/**/*.patch') }}
- name: Install JavaScript dependencies (npm install)
if: steps.cache-nodemodules.outputs.cache-hit != 'true'
run: npm install
- name: Run copy-fonts (if using cached node_modules)
if: steps.cache-nodemodules.outputs.cache-hit == 'true'
key: ${{ runner.os }}-build-${{ env.cache-name }}-node-v${{ steps.resolved-node-version.outputs.NODE_VERSION }}-${{ hashFiles('**/package-lock.json', 'patches/**/*.patch') }}

# Cache hit: node_modules is copied from a previous run. Run copy-fonts
- if: steps.cache-nodemodules.outputs.cache-hit == 'true'
name: Run copy-fonts (if using cached node_modules)
run: npm run copy-fonts

# Cache miss: Run npm install, which does copy-fonts as post-install step
- if: steps.cache-nodemodules.outputs.cache-hit != 'true'
name: Install JavaScript dependencies (npm install)
run: npm install

# Build the app!
- name: Build Prod
run: SKIP_TS_CHECK=true npm run build

# Run TypeScript Checks and ESLint
- name: Check TypeScript # Separated for visibility
run: npm run check-types
- name: Check ESLint, errors only
run: npm run lint -- --quiet

# Unit Tests
- name: Build Tests
run: npx webpack --config webpack/test.config.js

- name: Run Tests, with mocha-chrome
run: npx mocha-chrome test/tests.html --chrome-launcher.connectionPollInterval=5000
# This step takes less than 1 minute if it succeeds, but will hang for
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ jobs:
ports:
- 6379:6379
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install pip-tools
Expand Down
2 changes: 1 addition & 1 deletion .node-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.15.0
v20.17.0
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.15.0
v20.17.0
14 changes: 5 additions & 9 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,8 @@ RUN mkdir -p "${NGINX_STATIC_DIR}" && \
# jnm (or the current on-call sysadmin). Thanks.

RUN apt-get -qq update && \
apt-get -qq -y install ca-certificates curl gnupg && \
mkdir -p /etc/apt/keyrings && \
curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
| gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg && \
echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_16.x nodistro main" \
| tee /etc/apt/sources.list.d/nodesource.list && \
apt-get -qq update && \
apt-get -qq -y install curl && \
curl -fsSL https://deb.nodesource.com/setup_20.x | bash - && \
apt-get -qq -y install openjdk-17-jre && \
apt-get -qq -y install --no-install-recommends \
ffmpeg \
Expand All @@ -65,7 +60,8 @@ RUN apt-get -qq update && \
less \
libproj-dev \
locales \
nodejs=$(apt-cache show nodejs | grep -F 'Version: 16.15.0' | cut -f 2 -d ' ') \
# pin an exact Node version for stability. update this regularly.
nodejs=$(apt-cache show nodejs | grep -F 'Version: 20.17.0' | cut -f 2 -d ' ') \
postgresql-client \
procps \
rsync \
Expand Down Expand Up @@ -109,7 +105,7 @@ WORKDIR ${KPI_SRC_DIR}/
RUN rm -rf ${KPI_NODE_PATH} && \
mkdir -p "${TMP_DIR}/.npm" && \
npm config set cache "${TMP_DIR}/.npm" --global && \
npm install -g npm@8.5.5 && \
# to pin an exact npm version, see 'git blame' here
npm install -g check-dependencies@1 && \
rm -rf "${KPI_SRC_DIR}/jsapp/fonts" && \
rm -rf "${KPI_SRC_DIR}/jsapp/compiled" && \
Expand Down
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
"version": "2.0.0",
"description": "KoboToolbox frontend interface.",
"engines": {
"node": "16.15.0",
"npm": "8.5.5"
"node": "^20.17.0 || ^22.4.1"
},
"dependencies": {
"@fontsource/roboto": "^4.4.2",
Expand Down
13 changes: 13 additions & 0 deletions patches/mocha-chrome+2.2.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/node_modules/mocha-chrome/lib/client.js b/node_modules/mocha-chrome/lib/client.js
index 7629126..cd3d163 100644
--- a/node_modules/mocha-chrome/lib/client.js
+++ b/node_modules/mocha-chrome/lib/client.js
@@ -9,7 +9,7 @@ module.exports = async function connectClient(instance, log, options) {
return fs.readFileSync(filePath, 'utf-8');
}

- const client = await CDP({ port: instance.port });
+ const client = await CDP({ port: instance.port, host: '127.0.0.1' });
const { DOM, DOMStorage, Console, Network, Page, Runtime } = client;
const mochaOptions = `window.mochaOptions = ${JSON.stringify(options.mocha)}`;

38 changes: 20 additions & 18 deletions scripts/hints.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,18 @@ if (process.env.SKIP_TS_CHECK && tsCheckAffects.includes(hintName)) {
/*
NPM VERSION WARNING

Issue a warning if not running with npm 8.5.5, since the errors
that you get if you run a different version are not very obvious.
Issue a warning if there's a version mismatch between expected and user's
version of Node or npm, since it could cause strange difficulties.

- Help contributors who switched their Node version inadvertently.
- Support new contributors by printing more context / steps to remedy.
- Give the benefit of the doubt to developers who run a different node/npm
version on purpose. So, don't process.exit(1), even on npm install.
It is a bit redundant with the EBADENGINE warning issued from package.json,
but this script (originally written for a bumpy >=16.15.1 upgrade) provides
a bit of context.

Show on preinstall. Since it's easy to miss there, also show it on other
run scripts such as 'watch'.
*/
const ok_node = 'v16.15.0';
const ok_npm = '8.5.5';
const ok_node = 'v20.17.0';
const ok_npm = '10.8.2';

if (process.version !== ok_node) {
const blu = '\u001b[94m'; // bright blue
Expand All @@ -79,10 +78,10 @@ if (process.version !== ok_node) {
console.warn(`${blu}
--------------------------------------------------------------`);

console.warn(`${yel}
Are you running the supported version of Node and npm?
console.warn(`${nrm}
Are you running a supported version of Node and npm?
${nrm}
node ${ok_node}, npm@${ok_npm} supported`);
node ${yel}${ok_node}${nrm}, ${yel}npm@${ok_npm}${nrm} supported`);

// Let's be more helpful by running `npm --version` instead of making
// you do it.
Expand Down Expand Up @@ -110,21 +109,24 @@ if (process.version !== ok_node) {
// what causes the most problems.
if (wrongNpm) {
console.warn(`
Switch to a supported Node / npm version:
${blu}(This is probably OK, but it's helpful to
run the same version we're using in release.)${nrm}

Use Node v16.15.0, which comes with [email protected]
To switch to a supported Node / npm version:

Use Node ${ok_node}, which comes with npm@${ok_npm}
\`nvm use\` or \`fnm use\`

or \`npm install -g npm@8.5.5\`
or \`npm install -g npm@${ok_npm}\`
to change npm for your current Node`);

console.warn(`${red}
If you've run \`npm install\` with an unsupported npm version,${nrm}
there may be changes in node_modules and package-lock.json
console.warn(`${yel}
If you've run \`npm install\` with a different npm version,${nrm}
there could be changes in node_modules and package-lock.json

(1) Don't commit these changes to package-lock.json
(2) You may want to reset these changes and run
\`npm install\` again with 8.5.5
\`npm install\` again with ${ok_npm}
`);

// If you switch between Node projects and see this message often,
Expand Down