Skip to content

Add explicit version checks - #211

Merged
camillobruni merged 6 commits into
WebKit:mainfrom
camillobruni:2025-10-21_prescript
Oct 22, 2025
Merged

camillobruni merged 6 commits into
WebKit:mainfrom
camillobruni:2025-10-21_prescript

Conversation

@camillobruni

Copy link
Copy Markdown
Contributor
  • Add utils/version-check.mjs to warn against invalid nodejs versions
  • Add license headers to newly added files, utils and test files

@netlify

netlify Bot commented Oct 21, 2025 •

Copy link
Copy Markdown

✅ Deploy Preview for webkit-jetstream-preview ready!

Name Link
🔨 Latest commit 22e0ea7
🔍 Latest deploy log https://app.netlify.com/projects/webkit-jetstream-preview/deploys/68f8ec6bd0931f00088ea47f
😎 Deploy Preview https://deploy-preview-211--webkit-jetstream-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@camillobruni
camillobruni requested a review from danleh October 21, 2025 09:25

@danleh danleh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Regarding the version checks, is there a reason to not use the more builtin checking by adding a .npmrc file with engine-strict=true? AFAIU node already warns if the versions in engines are not fulfilled; with the additional config file it should error out (not tested, just read that at https://stackoverflow.com/questions/29349684/how-can-i-specify-the-required-node-js-version-in-package-json)

Yay to proper license headers :)

@camillobruni

Copy link
Copy Markdown
Contributor Author

I've tried the npmrc approach and I didn't work locally :/.

% npm run test:v8

> jetstream@3.0.0-alpha test:v8
> npm run test:prepare && node tests/run-shell.mjs --shell v8


> jetstream@3.0.0-alpha test:prepare
> npm run decompress -- --quiet


> jetstream@3.0.0-alpha decompress
> node utils/compress.mjs --decompress --keep --quiet

RexBench/FlightPlanner/waypoints.js.z
SeaMonster/inspector-json-payload.js.z
wasm/argon2/build/argon2.wasm.z
file:///Users/test/Documents/jetstream/tests/helper.mjs:1
import { styleText } from "node:util";
         ^^^^^^^^^
SyntaxError: The requested module 'node:util' does not provide an export named 'styleText'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)

Node.js v19.6.0

@camillobruni
camillobruni requested a review from danleh October 22, 2025 13:11
@danleh

danleh commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Ah well, let's go with the explicit version check script then (unless Keith or someone else has an idea).

@kmiller68 kmiller68 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM too. I don't know a better way to test node's version either. We can always remove this if we find a better way to test.

@camillobruni
camillobruni merged commit eda9143 into WebKit:main Oct 22, 2025
10 checks passed
@camillobruni
camillobruni deleted the 2025-10-21_prescript branch October 22, 2025 15:05
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.

3 participants