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

Move from node 16 to node >=18 #12711

Merged
merged 13 commits into from
Aug 28, 2023
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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: the Developing.md was updated to say developers should use >= 18.17 and < 21. But this line says >= 16. Why that discrepancy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we generally recommend the version range that you've mentioned, there is no technical reason for limiting it to that range. Though we know that < 16.0.0 doesn't work due to native dependencies. 18.17.0 is specifically the recommended minimum version because that's the version that our Electron version uses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recommend a certain version range in our documentation as we try to make sure it works for said range.

This engines.node field will have Yarn stop the build if you use anything outside the defined range, and I might have left it lax so that it's more convenient when switching between node versions for development purposes.

Most other NPM projects -including our deps- are also pretty lax with this range (click to expand).

But we could move the lower bound along the minimum recommended version.

At the very minimum we need to update this range based on the kind of constructs we use from NodeJS: we can't use new Node 18 syntax and claim that we require anything below Node 18 to run our code base. Although syntax-wise we're dependent on our TypeScript compilation target: No matter what TS syntax we use tsc will "dumb it down" for us in the output JS based on our configs. We're currently targeting ES2019 and it seems like Node 16 supports it very well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the long run it's in everyone's best interest to always use the latest Node LTS, but some might still lag behind so the range can accommodate for it somewhat.

Now I remember: Had I updated the range to >=18 for that release then all users of the framework would have had build issues because they most likely were still running on Node 16 at that point. So instead of breaking everyone on release I kept it that way so people can update when they're ready. It's not like the project will stop working on Node 16 right away :)

},
"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
Loading