restify / restify/node-restify

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

Open
#1,932 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
10.7k
Forks
975
Avg merge
1d 6h
Merged PRs (30d)
5

Description

  • 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 restify@9.0.0, 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	git@github.com:restify/node-restify.git (fetch)
origin	git@github.com: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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Inspect package.json and lib/request.js first, then run the reported require repro with the affected Node.js versions. Confirm the supported Node.js range and the compatibility impact of the node:process import; done means the engines declaration and actual import behavior agree, with the branch choice resolved.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.