Skip to content

Commit

Permalink
repo: upgrade from node 16 to node >=18 (#12711)
Browse files Browse the repository at this point in the history
The commit includes the following changes:

- update deps for node 18.x
- update breaking changes
- update workflows
- make sure node-gyp is installed
- update logger tests
- update docs
- force ipv4 and improve ipv6 support
- fix backend address messaging
- update backend cli options
- update runtime policy
- fix backend cli
- use node 18.x in gitpod image
- keep using node 16.x in workflow

Using Node 20.x it seems like Yarn complains about missing `node-gyp`.
It will actually shortcut the first call to `yarn install` when it is missing in order to automatically install it.

Instead we'll explicitly make sure that node-gyp is installed.
  • Loading branch information
paul-marechal authored Aug 28, 2023
1 parent 9001508 commit 2d31490
Show file tree
Hide file tree
Showing 19 changed files with 85 additions and 73 deletions.
8 changes: 5 additions & 3 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ jobs:
- name: Checkout
uses: actions/checkout@v3

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v3
with:
node-version: 16.x
node-version: 18.x
registry-url: 'https://registry.npmjs.org'

- name: Use Python 3.x
Expand All @@ -34,6 +34,7 @@ jobs:
- name: Install and Build
shell: bash
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000
./scripts/check_git_status.sh
env:
Expand All @@ -51,7 +52,7 @@ jobs:
fail-fast: false
matrix:
os: [windows-2019, ubuntu-latest, macos-11]
node: [16.x]
node: [16.x, 18.x, 20.x]

runs-on: ${{ matrix.os }}
timeout-minutes: 60
Expand All @@ -74,6 +75,7 @@ jobs:
- name: Install
shell: bash
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000
./scripts/check_git_status.sh
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/license-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
node: ['16.x']
node: ['18.x']
java: ['11']

runs-on: ${{ matrix.os }}
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/performance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ on:
jobs:
build-and-test-performance:
name: Performance Tests

runs-on: ubuntu-latest
timeout-minutes: 30

steps:
- name: Checkout
uses: actions/checkout@v3

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v3
with:
node-version: "16.x"
node-version: "18.x"
registry-url: "https://registry.npmjs.org"

- name: Use Python 3.x
Expand All @@ -28,6 +28,7 @@ jobs:
- name: Build
shell: bash
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000 --ignore-engines
yarn build:examples
env:
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:

jobs:
build-and-test-playwright:
name: Playwright Tests (ubuntu-latest, Node.js 16.x)
name: Playwright Tests (ubuntu-latest, Node.js 18.x)

runs-on: ubuntu-latest
timeout-minutes: 60
Expand All @@ -22,10 +22,10 @@ jobs:
- name: Checkout
uses: actions/checkout@v3

- name: Use Node.js "16.x"
- name: Use Node.js "18.x"
uses: actions/setup-node@v3
with:
node-version: "16.x"
node-version: "18.x"
registry-url: "https://registry.npmjs.org"

- name: Use Python 3.x
Expand All @@ -36,6 +36,7 @@ jobs:
- name: Build Browser
shell: bash
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000
yarn browser build
env:
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/publish-gh-pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ jobs:
with:
fetch-depth: 0 # To fetch all history for all branches and tags. (Will be required for caching with lerna: https://github.com/markuplint/markuplint/pull/111)

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v3
with:
node-version: '16.x'
node-version: '18.x'
registry-url: 'https://registry.npmjs.org'

- name: Use Python 3.x
Expand All @@ -32,6 +32,7 @@ jobs:

- name: Pre-npm-Publish
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000
env:
NODE_OPTIONS: --max_old_space_size=4096
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/translation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ jobs:
- name: Checkout
uses: actions/checkout@v3

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v3
with:
node-version: 16.x
node-version: 18.x
registry-url: "https://registry.npmjs.org"

- name: Use Python 3.x
Expand All @@ -26,6 +26,7 @@ jobs:
- name: Install and Build
shell: bash
run: |
yarn global add node-gyp
yarn --skip-integrity-check --network-timeout 100000
env:
NODE_OPTIONS: --max_old_space_size=4096
Expand Down
2 changes: 1 addition & 1 deletion .gitpod.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ RUN sudo apt-get update \

ENV SHELL=/bin/bash

ENV NODE_VERSION="16.14.0"
ENV NODE_VERSION="18.17.0"
RUN bash -c ". .nvm/nvm.sh \
&& nvm install $NODE_VERSION \
&& nvm use $NODE_VERSION \
Expand Down
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@
## History

- [Previous Changelogs](https://github.com/eclipse-theia/theia/tree/master/doc/changelogs/)
## v1.41.0 -

## v1.41.0

- [application-package] Quit Electron app when back end fails to start [#12778](https://github.com/eclipse-theia/theia/pull/12778) - Contributed on behalf of STMicroelectronics.
- [vscode] added support for tree checkbox api [#12836](https://github.com/eclipse-theia/theia/pull/12836) - Contributed on behalf of STMicroelectronics
- [core] Add `--dnsDefaultResultOrder <value>` CLI argument where `value` is one of `ipv4first`, `verbatim` or `nodeDefault`. It controls how domain names are resolved.

<a name="breaking_changes_1.41.0">[Breaking Changes:](#breaking_changes_1.41.0)</a>

- [deps] Bumped supported Node.js version from 16.x to >=18, you may need to update your environments.

## v1.40.0 - 07/27/2023

- [application-package] bumped the default supported VS Code API from `1.78.0` to `1.79.0` [#12764](https://github.com/eclipse-theia/theia/pull/12764) - Contributed on behalf of STMicroelectronics.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ BackendApplicationConfigProvider.set(${this.prettyStringify(this.pck.props.backe
const serverModule = require('./server');
const serverAddress = main.start(serverModule());
serverAddress.then(({ port, address }) => {
serverAddress.then(({ port, address, family }) => {
if (process && process.send) {
process.send({ port, address });
process.send({ port, address, family });
}
});
Expand Down
8 changes: 5 additions & 3 deletions dev-packages/cli/src/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ export default async function runTest(options: TestOptions): Promise<void> {
}
}
});

const server = await start();
await testPage.goto(`http://${server.address}:${server.port}`);
const { address, port } = await start();
const url = net.isIPv6(address)
? `http://[${address}]:${port}`
: `http://${address}:${port}`;
await testPage.goto(url);
}
4 changes: 2 additions & 2 deletions doc/Developing.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ For Windows instructions [click here](#building-on-windows).

## Prerequisites

- Node.js `>= 16.14.0` and `< 17`.
- Node.js `>= 18.17.0` and `< 21`.
- If you are interested in Theia's VS Code Extension support then you should use a Node version at least compatible with the one included in the version of Electron used by [VS Code](https://github.com/microsoft/vscode).
- [Yarn package manager](https://yarnpkg.com/en/docs/install) `>= 1.7.0` **AND** `< 2.x.x`.
- git (If you would like to use the Git-extension too, you will need to have git version 2.11.0 or higher.)
Expand Down Expand Up @@ -508,7 +508,7 @@ etc.) by opening `packages/<package name>/coverage/index.html`.

- Install [`scoop`](https://github.com/lukesampson/scoop#installation).
- Install [`nvm`](https://github.com/coreybutler/nvm-windows) with scoop: `scoop install nvm`.
- Install Node.js with `nvm`: `nvm install 16.15.1`, then use it: `nvm use 16.15.1`. You can list all available Node.js versions with `nvm list available` if you want to pick another version.
- Install Node.js with `nvm`: `nvm install 18.17.0`, then use it: `nvm use 18.17.0`. You can list all available Node.js versions with `nvm list available` if you want to pick another version.
- Install `yarn`: `scoop install yarn`.
- If you need to install `windows-build-tools`, see [`Installing Windows Build Tools`](#installing-windows-build-tools).
- If you run into problems with installing the required build tools, the `node-gyp` documentation offers a useful [guide](https://github.com/nodejs/node-gyp#on-windows) how to install the dependencies manually. The versions required for building Theia are:
Expand Down
3 changes: 2 additions & 1 deletion doc/runtime-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Version Support Policy

We try to support Node.js versions from 12 up to the current _Active LTS_ version.
We aim to support Node.js current _Active LTS_ version.

See https://nodejs.org/en/about/releases/ to see the status of Node.js versions.

Expand All @@ -15,6 +15,7 @@ Note that the Node.js version you should use depends on your own project's depen
- Follow Node.js LTS cadence and initiate the update when a new Node.js version becomes _Active LTS_.
- Use `@types/node` for the oldest supported Node version (backward compatibility).
- Update the CI matrix to include the new Node.js versions to support.
- Update the documentation referencing recommended Node versions.
- Update the CHANGELOG.

# Electron
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@
"version": "0.0.0",
"engines": {
"yarn": ">=1.7.0 <2",
"node": ">=16.14.0 <17"
"node": ">=16"
},
"resolutions": {
"**/@types/node": "16"
"**/@types/node": "18"
},
"devDependencies": {
"@types/chai": "4.3.0",
"@types/chai-spies": "1.0.3",
"@types/chai-string": "^1.4.0",
"@types/jsdom": "^21.1.1",
"@types/node": "16",
"@types/node": "18",
"@types/sinon": "^10.0.6",
"@types/temp": "^0.9.1",
"@types/uuid": "^7.0.3",
Expand Down
35 changes: 30 additions & 5 deletions packages/core/src/node/backend-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import * as dns from 'dns';
import * as path from 'path';
import * as http from 'http';
import * as https from 'https';
Expand All @@ -29,13 +30,16 @@ import { AddressInfo } from 'net';
import { ApplicationPackage } from '@theia/application-package';
import { ProcessUtils } from './process-utils';

export type DnsResultOrder = 'ipv4first' | 'verbatim' | 'nodeDefault';

const APP_PROJECT_PATH = 'app-project-path';

const TIMER_WARNING_THRESHOLD = 50;

const DEFAULT_PORT = environment.electron.is() ? 0 : 3000;
const DEFAULT_HOST = 'localhost';
const DEFAULT_SSL = false;
const DEFAULT_DNS_DEFAULT_RESULT_ORDER: DnsResultOrder = 'ipv4first';

export const BackendApplicationServer = Symbol('BackendApplicationServer');
/**
Expand Down Expand Up @@ -107,6 +111,7 @@ export class BackendApplicationCliContribution implements CliContribution {

port: number;
hostname: string | undefined;
dnsDefaultResultOrder: DnsResultOrder = DEFAULT_DNS_DEFAULT_RESULT_ORDER;
ssl: boolean | undefined;
cert: string | undefined;
certkey: string | undefined;
Expand All @@ -119,6 +124,12 @@ export class BackendApplicationCliContribution implements CliContribution {
conf.option('cert', { description: 'Path to SSL certificate.', type: 'string' });
conf.option('certkey', { description: 'Path to SSL certificate key.', type: 'string' });
conf.option(APP_PROJECT_PATH, { description: 'Sets the application project directory', default: this.appProjectPath() });
conf.option('dnsDefaultResultOrder', {
type: 'string',
description: 'Configure Node\'s DNS resolver default behavior, see https://nodejs.org/docs/latest-v18.x/api/dns.html#dnssetdefaultresultorderorder',
choices: ['ipv4first', 'verbatim', 'nodeDefault'],
default: DEFAULT_DNS_DEFAULT_RESULT_ORDER
});
}

setArguments(args: yargs.Arguments): void {
Expand All @@ -128,6 +139,7 @@ export class BackendApplicationCliContribution implements CliContribution {
this.cert = args.cert as string;
this.certkey = args.certkey as string;
this.projectPath = args[APP_PROJECT_PATH] as string;
this.dnsDefaultResultOrder = args.dnsDefaultResultOrder as DnsResultOrder;
}

protected appProjectPath(): string {
Expand Down Expand Up @@ -234,9 +246,13 @@ export class BackendApplication {
this.app.use(...handlers);
}

async start(aPort?: number, aHostname?: string): Promise<http.Server | https.Server> {
const hostname = aHostname !== undefined ? aHostname : this.cliParams.hostname;
const port = aPort !== undefined ? aPort : this.cliParams.port;
async start(port?: number, hostname?: string): Promise<http.Server | https.Server> {
hostname ??= this.cliParams.hostname;
port ??= this.cliParams.port;

if (this.cliParams.dnsDefaultResultOrder !== 'nodeDefault') {
dns.setDefaultResultOrder(this.cliParams.dnsDefaultResultOrder);
}

const deferred = new Deferred<http.Server | https.Server>();
let server: http.Server | https.Server;
Expand Down Expand Up @@ -279,8 +295,10 @@ export class BackendApplication {
});

server.listen(port, hostname, () => {
const scheme = this.cliParams.ssl ? 'https' : 'http';
console.info(`Theia app listening on ${scheme}://${hostname || 'localhost'}:${(server.address() as AddressInfo).port}.`);
// address should be defined at this point
const address = server.address()!;
const url = typeof address === 'string' ? address : this.getHttpUrl(address, this.cliParams.ssl);
console.info(`Theia app listening on ${url}.`);
deferred.resolve(server);
});

Expand All @@ -301,6 +319,13 @@ export class BackendApplication {
return this.stopwatch.startAsync('server', 'Finished starting backend application', () => deferred.promise);
}

protected getHttpUrl({ address, port, family }: AddressInfo, ssl?: boolean): string {
const scheme = ssl ? 'https' : 'http';
return family.toLowerCase() === 'ipv6'
? `${scheme}://[${address}]:${port}`
: `${scheme}://${address}:${port}`;
}

protected onStop(): void {
console.info('>>> Stopping backend contributions...');
for (const contrib of this.contributionsProvider.getContributions()) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/node/logger-cli-contribution.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('log-level-cli-contribution', () => {

const args: yargs.Arguments = yargs.parse(['--log-config', file.path]);
await cli.setArguments(args);
sinon.assert.calledWithMatch(consoleErrorSpy, 'Unexpected token { in JSON at position 1');
sinon.assert.calledWithMatch(consoleErrorSpy, 'Error reading log config file');
});

// Skip this test because it is flaky, sometimes we don't receive the event.
Expand Down
7 changes: 1 addition & 6 deletions packages/core/src/node/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ process.on('unhandledRejection', (reason, promise) => {
throw reason;
});

export interface Address {
readonly port: number;
readonly address: string;
}

export async function start(serverModule: MaybePromise<http.Server | https.Server>): Promise<Address> {
export async function start(serverModule: MaybePromise<http.Server | https.Server>): Promise<AddressInfo> {
const server = await serverModule;
return server.address() as AddressInfo;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"is-electron": "^2.2.0",
"jsonc-parser": "^2.2.0",
"lodash.clonedeep": "^4.5.0",
"macaddress": "^0.2.9",
"macaddress": "^0.5.3",
"mime": "^2.4.4",
"ps-tree": "^1.2.0",
"semver": "^7.5.4",
Expand Down
Loading

0 comments on commit 2d31490

Please sign in to comment.