node-restify icon indicating copy to clipboard operation
node-restify copied to clipboard

package.json's "engines" says `"node": ">=10.0.0"` but actual base version is v14.18.0

Open trentm opened this issue 2 years ago • 2 comments

  • [x] Used appropriate template for the issue type
  • [x] Searched both open and closed issues for duplicates of this issue
  • [x] Title adequately and concisely reflects the feature or the bug

Restify Version: 9.0.0 Node.js Version: any

Expected behaviour

That the "engines" field of "package.json" accurately says what versions of node.js restify works with.

Actual behaviour

The "engines" for [email protected], and in the current repo tip, says "node": ">=10.0.0". However, with anything less that node v14.8.0 importing restify crashes:

% node --version
v14.17.6

% git remote -v
origin	[email protected]:restify/node-restify.git (fetch)
origin	[email protected]:restify/node-restify.git (push)
% git log --oneline -1
426f7e9 (HEAD -> master, origin/master, origin/HEAD) Update example to allow downgrading to http1

% node -e 'require("./")'
internal/modules/cjs/loader.js:892
  throw err;
  ^

Error: Cannot find module 'node:process'
Require stack:
- /Users/trentm/src/node-restify/lib/request.js
- /Users/trentm/src/node-restify/lib/server.js
- /Users/trentm/src/node-restify/lib/index.js
- /Users/trentm/src/node-restify/[eval]
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:889:15)
    at Function.Module._load (internal/modules/cjs/loader.js:745:27)
    at Module.require (internal/modules/cjs/loader.js:961:19)
    at require (internal/modules/cjs/helpers.js:92:18)
    at Object.<anonymous> (/Users/trentm/src/node-restify/lib/request.js:5:25)
    at Module._compile (internal/modules/cjs/loader.js:1072:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1101:10)
    at Module.load (internal/modules/cjs/loader.js:937:32)
    at Function.Module._load (internal/modules/cjs/loader.js:778:12)
    at Module.require (internal/modules/cjs/loader.js:961:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/trentm/src/node-restify/lib/request.js',
    '/Users/trentm/src/node-restify/lib/server.js',
    '/Users/trentm/src/node-restify/lib/index.js',
    '/Users/trentm/src/node-restify/[eval]'
  ]
}

Repro case

(Shown above.)

Cause

The issue is the usage of "Core modules" (i.e. the node:-prefix) in "lib/request.js":

const { emitWarning } = require('node:process');

Are you willing and able to fix this?

Yes. A few questions, though:

  • IIUC, The intent of commit c15111fb2862705d49dbd6cf60612069f13adb8d was to drop node v10 and v12 support, so would a PR to update "engines" be acceptable?

  • Support for all of node v14 (back to v14.0.0) could be restored with the following change.

diff --git a/lib/request.js b/lib/request.js
index a4c54d0..d67dc96 100644
--- a/lib/request.js
+++ b/lib/request.js
@@ -2,7 +2,7 @@

 'use strict';

-const { emitWarning } = require('node:process');
+const { emitWarning } = require('process');

or even just this change:

-const { emitWarning } = require('node:process');
+const { emitWarning } = process;

Is that desired? I don't have a strong preference.

  • It seems that the "9.x" branch is out of sync. Should I make my PR against the "master" branch, and ignore "9.x"?

PR https://github.com/restify/node-restify/pull/1923 is somewhat related to this. Thanks.

trentm avatar Nov 17 '22 20:11 trentm

Resolved in #1923 . Covered by #1921 .

pinko-fowle avatar Nov 22 '22 21:11 pinko-fowle

Removing const { emitWarning } = require('node:process'); completely will not only fix the github action error in #1929 but also resolve this issue making restify truely support >14.0.0

acommodari avatar Nov 28 '22 15:11 acommodari