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 <sam@elide.dev>
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 <sam@elide.dev>
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 <sam@elide.dev>
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 <sam@elide.dev>
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 <sam@elide.dev> * chore: version bump → `1.0.3` Signed-off-by: Sam Gammon <sam@elide.ventures> --------- Signed-off-by: Sam Gammon <sam@elide.dev> Signed-off-by: Sam Gammon <sam@elide.ventures>
|
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 <sam@elide.ventures>
sgammon
added a commit
to sgammon/hashlock
that referenced
this pull request
Apr 15, 2024
* fix: postinstall script Signed-off-by: Sam Gammon <sam@elide.ventures> * chore: version bump → `1.0.4` Signed-off-by: Sam Gammon <sam@elide.ventures> * chore: update to commander.js `develop` branch Adopt changes from tj/commander.js#2170 Signed-off-by: Sam Gammon <sam@elide.ventures> * fix: disable distcheck (tmp) Signed-off-by: Sam Gammon <sam@elide.ventures> * chore: update all node deps Signed-off-by: Sam Gammon <sam@elide.ventures> --------- Signed-off-by: Sam Gammon <sam@elide.ventures>
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.