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

misc: drop node 16 support #15290

Merged
merged 11 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ jobs:
GITHUB_CONTEXT_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
GITHUB_CONTEXT_BASE_SHA: ${{ github.event.before }}

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn type-check
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/cron-weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ jobs:
steps:
- name: git clone
uses: actions/checkout@v2
- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x
- run: yarn --frozen-lockfile

- run: yarn mocha --testMatch=third-party/chromium-synchronization/*-test.js
12 changes: 6 additions & 6 deletions .github/workflows/devtools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ jobs:
with:
path: lighthouse

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- name: Generate cache hash
run: bash $GITHUB_WORKSPACE/lighthouse/.github/scripts/generate-devtools-hash.sh > cdt-test-hash.txt
Expand Down Expand Up @@ -91,10 +91,10 @@ jobs:
with:
path: lighthouse

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- run: yarn --frozen-lockfile --network-timeout 1000000
working-directory: ${{ github.workspace }}/lighthouse
Expand Down Expand Up @@ -136,10 +136,10 @@ jobs:
with:
path: lighthouse

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- name: Load build artifacts
id: devtools-build-artifacts
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/package-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ jobs:
- name: git clone
uses: actions/checkout@v2

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x
registry-url: https://registry.npmjs.org/
- run: yarn --frozen-lockfile

Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ jobs:
# Depth of at least 2 for codecov coverage diffs. See https://github.com/GoogleChrome/lighthouse/pull/12079
fetch-depth: 2

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- name: Define ToT chrome path
if: matrix.chrome-channel == 'ToT'
Expand Down Expand Up @@ -138,10 +138,10 @@ jobs:
- name: git clone
uses: actions/checkout@v2

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
unit:
strategy:
matrix:
node: ['16', '18']
node: ['18', '20']
fail-fast: false
runs-on: ubuntu-latest
name: node ${{ matrix.node }}
Expand Down Expand Up @@ -93,10 +93,10 @@ jobs:
- name: git clone
uses: actions/checkout@v2

- name: Use Node.js 16.x
- name: Use Node.js 18.x
uses: actions/setup-node@v1
with:
node-version: 16.x
node-version: 18.x

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report
Expand Down
3 changes: 2 additions & 1 deletion cli/test/cli/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ describe('CLI Tests', function() {
const ret = spawnSync('node', [indexPath, 'https://www.google.com',
'--extra-headers', '{notjson}'], {encoding: 'utf8'});

assert.ok(ret.stderr.includes('Unexpected token'));
// This message was changed in Node 20, check for old and new versions.
assert.ok(/(Unexpected token|Expected property name)/.test(ret.stderr));
assert.equal(ret.status, 1);
});
});
Expand Down
18 changes: 9 additions & 9 deletions cli/test/fixtures/static-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ class Server {
headers['X-TotalFetchedSize'] = Buffer.byteLength(data) + JSON.stringify(headers).length;
}

response.writeHead(statusCode, headers);
response.statusCode = statusCode;
for (const [name, value] of Object.entries(headers)) {
response.setHeader(name, value);
}
const encoding = charset === 'UTF-8' ? 'utf-8' : 'binary';

// Delay the response
Expand All @@ -188,9 +191,8 @@ class Server {
<h1>Smoke test fixtures</h1>
${fixturePaths.map(p => `<a href=${encodeURI(p)}>${escape(p)}</a>`).join('<br>')}
`;
response.writeHead(200, {
'Content-Security-Policy': `default-src 'none';`,
});
response.statusCode = 200;
response.setHeader('Content-Security-Policy', `default-src 'none';`);
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm can't find any info in the docs, but for some reason response.writeHead will end the response in Node 20 and send it to the user. We want to be able to amend the headers and status code later in execution (e.g. if there was a redirect) so we should use setHeader and statusCode = now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sendResponse(200, html);
return;
}
Expand Down Expand Up @@ -227,16 +229,14 @@ class Server {
function sendRedirect(url) {
// Redirects can only contain ASCII characters.
if (url.split('').some(char => char.charCodeAt(0) > 256)) {
response.writeHead(500);
response.statusCode = 500;
response.write(`Invalid redirect URL: ${url}`);
response.end();
return;
}

const headers = {
Location: url,
};
response.writeHead(302, headers);
response.statusCode = 302;
response.setHeader('Location', url);
response.end();
}

Expand Down
2 changes: 1 addition & 1 deletion cli/test/smokehouse/lighthouse-runners/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ async function runBundledLighthouse(url, config, testRunnerOptions) {
const logLevel = testRunnerOptions.isDebug ? 'verbose' : 'info';

// Puppeteer is not included in the bundle, we must create the page here.
const browser = await puppeteer.connect({browserURL: `http://localhost:${port}`});
const browser = await puppeteer.connect({browserURL: `http://127.0.0.1:${port}`});
Copy link
Member Author

@adamraine adamraine Jul 19, 2023

Choose a reason for hiding this comment

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

Node http/fetch no longer autoresolves localhost to the ipv4 address nodejs/node#40702

I guess it does in Node 20 though so this is only an issue for Node 18

const page = await browser.newPage();
const runnerResult = await lighthouse(url, {port, logLevel}, config, page);
if (!runnerResult) throw new Error('No runnerResult');
Expand Down
4 changes: 2 additions & 2 deletions core/test/config/config-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ describe('.resolveGathererToDefn', () => {
it('throws but not for missing gatherer when it has a node dependency error', async () => {
const resultPromise =
resolveGathererToDefn('../fixtures/invalid-gatherers/require-error.js', [], moduleDir);
await expect(resultPromise).rejects.toThrow(/Cannot find module/);
await expect(resultPromise).rejects.toThrow(/no such file or directory/);
Copy link
Member Author

@adamraine adamraine Jul 19, 2023

Choose a reason for hiding this comment

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

The "Cannot find module" text came from an error constructed by quibble. Updates in v0.7.0 of quibble appear to surface the actual node error instead. Good change IMO.

});
});

Expand Down Expand Up @@ -502,7 +502,7 @@ describe('.resolveAuditsToDefns', () => {
const resultPromise = resolveAuditsToDefns([
'../fixtures/invalid-audits/require-error.js',
], moduleDir);
await expect(resultPromise).rejects.toThrow(/Cannot find module/);
await expect(resultPromise).rejects.toThrow(/no such file or directory/);
});
});

Expand Down
8 changes: 6 additions & 2 deletions core/test/gather/gatherers/source-maps-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,17 @@ describe('SourceMaps gatherer', () => {
{
scriptUrl: mapsAndEvents[0].script.name,
sourceMapUrl: mapsAndEvents[0].script.sourceMapURL,
errorMessage: 'SyntaxError: Unexpected token { in JSON at position 1',
// This message was changed in Node 20, check for old and new versions.
// eslint-disable-next-line max-len
errorMessage: expect.stringMatching(/(Expected property name|Unexpected token).*at position 1/),
map: undefined,
},
{
scriptUrl: mapsAndEvents[1].script.name,
sourceMapUrl: undefined,
errorMessage: 'SyntaxError: Unexpected token ; in JSON at position 2',
// This message was changed in Node 20, check for old and new versions.
// eslint-disable-next-line max-len
errorMessage: expect.stringMatching(/(Unexpected non-whitespace|Unexpected token).*at position 2/),
map: undefined,
},
]);
Expand Down
8 changes: 4 additions & 4 deletions docs/headless-chrome.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
Setup:

```sh
# Lighthouse requires Node 16 LTS (16.x) or later.
curl -sL https://deb.nodesource.com/setup_14.x | sudo -E bash - &&\
# Lighthouse requires Node 18 LTS (18.x) or later.
curl -sL https://deb.nodesource.com/setup_18.x | sudo -E bash - &&\
sudo apt-get install -y nodejs npm

# get chromium (stable)
Expand All @@ -27,8 +27,8 @@ lighthouse --chrome-flags="--headless" https://github.com
Alternatively, you can run full Chrome + xvfb instead of headless mode. These steps worked on Debian Jessie:

```sh
# get node 16
curl -sL https://deb.nodesource.com/setup_16.x | sudo -E bash -
# get node 18
curl -sL https://deb.nodesource.com/setup_18.x | sudo -E bash -
sudo apt-get install -y nodejs npm

# get chromium (stable) and Xvfb
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"smokehouse": "./cli/test/smokehouse/frontends/smokehouse-bin.js"
},
"engines": {
"node": ">=16.16"
"node": ">=18"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you try on 18.0.0?

Copy link
Member Author

@adamraine adamraine Jul 27, 2023

Choose a reason for hiding this comment

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

Good point, I got some failures locally on 18.0.0 so I'll add a minor version

},
"scripts": {
"prepack": "yarn build-report --standalone --flow --esm && yarn build-types",
Expand Down Expand Up @@ -172,7 +172,7 @@
"rollup-plugin-polyfill-node": "^0.12.0",
"tabulator-tables": "^4.9.3",
"terser": "^5.18.2",
"testdouble": "^3.17.2",
"testdouble": "^3.18.0",
"typed-query-selector": "^2.6.1",
"typescript": "^5.0.4",
"wait-for-expect": "^3.0.2",
Expand Down
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ The Chrome extension was available prior to Lighthouse being available in Chrome

The Node CLI provides the most flexibility in how Lighthouse runs can be configured and reported. Users who want more advanced usage, or want to run Lighthouse in an automated fashion should use the Node CLI.

_Lighthouse requires Node 16 LTS (16.x) or later._
_Lighthouse requires Node 18 LTS (18.x) or later._

**Installation**:

Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6001,10 +6001,10 @@ queue-tick@^1.0.1:
resolved "https://registry.yarnpkg.com/queue-tick/-/queue-tick-1.0.1.tgz#f6f07ac82c1fd60f82e098b417a80e52f1f4c142"
integrity sha512-kJt5qhMxoszgU/62PLP1CJytzd2NKetjSRnyuj31fDd3Rlcz3fzlFdFLD1SItunPwyqEOkca6GbV612BWfaBag==

quibble@^0.6.17:
version "0.6.17"
resolved "https://registry.yarnpkg.com/quibble/-/quibble-0.6.17.tgz#1c47d40c4ee670fc1a5a4277ee792ca6eec8f4ca"
integrity sha512-uybGnGrx1hAhBCmzmVny+ycKaS5F71+q+iWVzbf8x/HyeEMDGeiQFVjWl1zhi4rwfTHa05+/NIExC4L5YRNPjQ==
quibble@^0.7.0:
version "0.7.0"
resolved "https://registry.yarnpkg.com/quibble/-/quibble-0.7.0.tgz#fd7f7f03ed4c2682b55523433af5e66bc26bde9f"
integrity sha512-uiqtYLo6p6vWR/G3Ltsg0NU1xw43RcNGadYP+d/DF3zLQTyOt8uC7L2mmcJ97au1QE1YdmCD+HVIIq/RGtkbWA==
dependencies:
lodash "^4.17.21"
resolve "^1.22.1"
Expand Down Expand Up @@ -6827,13 +6827,13 @@ test-exclude@^6.0.0:
glob "^7.1.4"
minimatch "^3.0.4"

testdouble@^3.17.2:
version "3.17.2"
resolved "https://registry.yarnpkg.com/testdouble/-/testdouble-3.17.2.tgz#a7d624c2040453580b4a636b3f017bf183a8f487"
integrity sha512-oRrk1DJISNoFr3aaczIqrrhkOUQ26BsXN3SopYT/U0GTvk9hlKPCEbd9R2uxkcufKZgEfo9D1JAB4CJrjHE9cw==
testdouble@^3.18.0:
version "3.18.0"
resolved "https://registry.yarnpkg.com/testdouble/-/testdouble-3.18.0.tgz#850a04d00045a52cd08c99cb69aea6845fe86587"
integrity sha512-awRay/WxNHYz0SJrjvvg1xE4QQkbKgWFN1VNhhb132JSO2FSWUW4cebUtD0HjWWwrvpN3uFsVeaUhwpmVlzlkg==
dependencies:
lodash "^4.17.21"
quibble "^0.6.17"
quibble "^0.7.0"
stringify-object-es5 "^2.5.0"
theredoc "^1.0.0"

Expand Down