fix: make require("cliui") work as expected for CJS#171
Merged
Conversation
Member
|
The documentation is a bit unclear about whether it should be needed, but when I was looking at Reference: https://nodejs.org/docs/latest/api/modules.html#loading-ecmascript-modules-using-require |
Member
|
I did some simple sanity checking. bun and deno don't currently support the (recent) variant syntax, it is a node special. But didn't cause any problems as such. // def.cjs
const def = require('./lib.mjs');
console.log({def});// lib.mjs
const defaultValue = "DEFAULT";
export default defaultValue
export {defaultValue as 'module.exports'};$ node def.cjs
{ def: 'DEFAULT' }$ deno run --allow-read def.cjs
{
def: [Module: null prototype] {
__esModule: true,
default: "DEFAULT",
"module.exports": "DEFAULT",
}
}$ bun run def.cjs
{
def: Module {
default: "DEFAULT",
"module.exports": "DEFAULT",
},
} |
Member
|
There is an issue open again Deno noting the new require(esm): denoland/deno#26649 |
Member
|
No problems with running an esm entry point. // modern.mjs
import def from './lib.mjs';
console.log({def});$ node modern.mjs
{ def: 'DEFAULT' }
$ deno run --allow-read modern.mjs
{ def: "DEFAULT" }
$ bun run --allow-read modern.mjs
{
def: "DEFAULT",
} |
This file contains hidden or 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
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.
@shadowspawn this uses the suggestion you gave in yargs adding:
Which seemed to make things work pretty well for both CJS and ESM:
Thoughts? Do you think this hack (
export {ui as 'module.exports'};) is reasonable, or is it going to bite us one day.BEGIN_COMMIT_OVERRIDE
fix: make require("cliui") work as expected for CJS
END_COMMIT_OVERRIDE