fix: make generated binding loaders Node 12 compatible#1
Open
NullVoxPopuli-ai-agent wants to merge 1 commit into
Open
fix: make generated binding loaders Node 12 compatible#1NullVoxPopuli-ai-agent wants to merge 1 commit into
NullVoxPopuli-ai-agent wants to merge 1 commit into
Conversation
The NAPI-RS-generated binding loaders are not parseable by Node 12,
which the packages still declare support for (engines: ">= 12"):
- `require('node:fs')` — the `node:` scheme in CommonJS `require()`
is only supported on Node >= 14.18 / 16, so it throws on Node 12.
- `process.report?.getReport` / `process.config?.variables?.…` —
optional chaining (`?.`) landed in Node 14, so Node 12 fails to
parse the module entirely.
Downlevel both to plain `require('fs')` and `&&` guards so the loaders
load on every Node version the packages claim to support. Applies to
all six native binding loaders.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on top of napi-rs#1147 (targets the
patch-1branch, notmain).napi-rs#1147 expands the CI matrix to include Node 12. The first thing that breaks on Node 12 isn't the binding — it's the NAPI-RS-generated JS loader itself failing to parse/load, even though every package still declares
engines: { "node": ">= 12" }.Two incompatibilities, both emitted by
@napi-rs/cli3.x:const { readFileSync } = require('node:fs')node:scheme in CommonJSrequire()is only supported on Node ≥ 14.18 / 16 → throwsCannot find module 'node:fs'process.report?.getReport/process.config?.variables?.…?.) landed in Node 14 → Node 12 fails to parse the module entirelyThis downlevels both to plain
require('fs')and&&guards across all six native binding loaders (argon2,bcrypt,crc32,jieba,jsonwebtoken,xxhash). The output still parses cleanly at ES2019 (verified with acornecmaVersion: 2019) and is unchanged behaviourally on newer Node.Caveat — the WASI job
napi-rs#1147 expands the matrix on
test-wasi-nodejs. WASI (wasm32-wasip1-threads) needs Node ≥ 18 to actually run, and the*.wasi.cjsloaders additionally use??+require('node:wasi'). So this change makes the native loaders Node-12-safe (honoringengines >= 12), but it does not make the WASI test itself pass on 12/14/16 — that job will still need its matrix narrowed to Node ≥ 18.Note on the source
These files are auto-generated by
@napi-rs/cli; the real fix belongs in its loader template (it hardcodesnode:fs+?.). This patches the committed output so the declared>= 12support holds today.