Fix: Use node-prefixed requires for builtins
#2170
Merged
Conversation
This file contains 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
Fixes and closes tj#2169 by using Node-prefixed imports. Signed-off-by: Sam Gammon <[email protected]>
sgammon
commented
Mar 31, 2024
Comment on lines
-1
to
+5
| const EventEmitter = require('events').EventEmitter; | ||
| const childProcess = require('child_process'); | ||
| const path = require('path'); | ||
| const fs = require('fs'); | ||
| const process = require('process'); | ||
| const EventEmitter = require('node:events').EventEmitter; | ||
| const childProcess = require('node:child_process'); | ||
| const path = require('node:path'); | ||
| const fs = require('node:fs'); | ||
| const process = require('node:process'); |
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Mar 31, 2024
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Mar 31, 2024
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Mar 31, 2024
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Mar 31, 2024
* feat: support all runtimes - feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]> * chore: version bump → `1.0.3` Signed-off-by: Sam Gammon <[email protected]> --------- Signed-off-by: Sam Gammon <[email protected]> Signed-off-by: Sam Gammon <[email protected]>
|
Commander v12 requires node v18 or higher, so node compatibility for using the Line 77 in 83c3f4e |
|
This issue only shows up because of bundling then running with Deno, and the change to fix it is only likely to break similar workflows involving transpiling and bundling tools not supporting the "node:" prefix for built-in modules. The
So I think normal uses of Commander won't be affected by this change, and if it breaks some more exotic uses then we will roll it back. |
shadowspawn
added
the
pending release
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Apr 15, 2024
Adopt changes from tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Apr 15, 2024
* fix: postinstall script Signed-off-by: Sam Gammon <[email protected]> * chore: version bump → `1.0.4` Signed-off-by: Sam Gammon <[email protected]> * chore: update to commander.js `develop` branch Adopt changes from tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]> * fix: disable distcheck (tmp) Signed-off-by: Sam Gammon <[email protected]> * chore: update all node deps Signed-off-by: Sam Gammon <[email protected]> --------- Signed-off-by: Sam Gammon <[email protected]>
|
Released in v12.1.0 |
shadowspawn
removed
the
pending release
|
@shadowspawn Thank you for the ping! I'll go ahead and adopt. 🥳 |
This was referenced Jun 19, 2024
This was referenced Sep 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request
Problem
I want to use a Commander-based CLI tool from Deno, as explained in #2169
Solution
Fixes and closes #2169 by using Node-prefixed imports.
Note: Node-prefixed imports are supported on Node 16+, so this may break older apps on very old versions of Node. As a result I would suggest a major version bump to signal a compatibility change. Node 16 has reached EOL, so people really should be upgrading if they can.
No code was changed in Commander itself; just imports.
ChangeLog
node:-prefixed requires; this may break compatibility with older versions of Node (up to 15). Please upgrade Node, bundle your app with rewriting to remove thenode:prefix on requires, or avoid upgrading Commander if Node cannot be upgraded.