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

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

Open
3 tasks done
trentm opened this issue Nov 17, 2022 · 2 comments
Open
3 tasks done

Comments

@trentm
Copy link
Contributor

trentm commented Nov 17, 2022

  • Used appropriate template for the issue type
  • Searched both open and closed issues for duplicates of this issue
  • 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 c15111f 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 #1923 is somewhat related to this.
Thanks.

@pinko-fowle
Copy link

Resolved in #1923 . Covered by #1921 .

@acommodari
Copy link

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

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

No branches or pull requests

3 participants