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

exception thrown when yargs.parsed.newAliases is undefined #1144

Closed
oscarocornejo opened this issue Jun 6, 2018 · 17 comments · Fixed by #1412
Closed

exception thrown when yargs.parsed.newAliases is undefined #1144

oscarocornejo opened this issue Jun 6, 2018 · 17 comments · Fixed by #1412
Labels

Comments

@oscarocornejo
Copy link

Found something of an issue when using yargs.showHelp()
L761

it seems that when self._parseArgs(processArgs) is called, it is not saving the results to self.parsed

if you put a breakpoint on this line: L1012, you will see that the self.parsed property contains the appropriate items.

however, after this function is done, on L762,
self.parsed is set back to false instead of an object. this leads to their being a parse error:

TypeError: Cannot convert undefined or null to object
.concat(Object.keys(yargs.parsed.newAliases) || [])

under function: usage.showHelp( L240

Thanks! 👍

@mleguen
Copy link
Member

mleguen commented Jun 19, 2019

I just ran into this when trying to answer @varl 's question in #1137.

The problem is not parsed being overwritten, but the returned value of require("yargs") having its properties (not its functions) frozen at their inital value (false for parsed).

So:

const yargs = require("yargs")
yargs.parse()
console.log(yargs.parsed)

returns false, whereas:

const yargs = require("yargs")
y = yargs.parse()
console.log(y)

returns the expected parsed object.

This is due to the way singletonify() in index.js is written.

I'll try to correct this behavior.

@mleguen
Copy link
Member

mleguen commented Jun 26, 2019

I am unable to reproduce the issue, so I was probably wrong when thinking it was related to require("yargs") properties being frozen.

@oscarocornejo I know it's been a while, but could you please give us some sample code reproducing the issue?

@bcoe
Copy link
Member

bcoe commented Jul 30, 2019

@oscarocornejo, with @mleguen's work investigating this issue, I ultimately opted to deprecated accessing yargs.parsed, which has issues when used in conjunction with commands (which require tracking state across multiple invocations of the parser).

Do you think you can instead use yargs.parse() or yargs.argv's return value?

@oscarocornejo
Copy link
Author

That should be fine, thanks guys 👍

@mleguen
Copy link
Member

mleguen commented Aug 14, 2019

Closing, as self.parsed is officially no longer supported since 14.0.0.

@mleguen mleguen closed this as completed Aug 14, 2019
@gajus
Copy link
Contributor

gajus commented Aug 21, 2019

I am seemingly getting the same error in 14.0.0.

I don't have much to share other than this is how I intialize the code:

yargs
  .env('ADM')
  .commandDir('commands')
  .help()
  .wrap(80)
  .parse();

This is the error that I get:

(node:92950) UnhandledPromiseRejectionWarning: TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Object.help (/Users/gajus/Documents/dev/applaudience/data-manager/node_modules/yargs/lib/usage.js:240:22)
    at Object.self.showHelp (/Users/gajus/Documents/dev/applaudience/data-manager/node_modules/yargs/lib/usage.js:432:15)
    at Object.Yargs.self.showHelp (/Users/gajus/Documents/dev/applaudience/data-manager/node_modules/yargs/yargs.js:790:11)
    at Object.fail (/Users/gajus/Documents/dev/applaudience/data-manager/node_modules/yargs/lib/usage.js:50:17)
    at /Users/gajus/Documents/dev/applaudience/data-manager/node_modules/yargs/lib/command.js:239:36
    at processTicksAndRejections (internal/process/task_queues.js:85:5)

@gajus
Copy link
Contributor

gajus commented Aug 21, 2019

.concat(Object.keys(yargs.parsed.newAliases) || [])

Here yargs.parsed.newAliases is undefined when this happens.

@gajus
Copy link
Contributor

gajus commented Aug 21, 2019

My builder is simply:

export const builder = (yargs: Object): void => {
  yargs
    .options({
      'cinema-data-scraper-sources-path': {
        demand: true,
        description: 'Path to the directory that includes the cinema data scraper sources.',
        type: 'string',
      },
      // [..]
    });
};

@bcoe
Copy link
Member

bcoe commented Aug 22, 2019

@gajus mind opening a new issue calling out this specific error?

@bcoe bcoe changed the title self.parsed being over written in yargs.js ln:761 exception thrown when yargs.parsed.newAliases is undefined Aug 22, 2019
@bcoe bcoe added the p0 label Aug 22, 2019
@bcoe bcoe reopened this Aug 22, 2019
@bcoe
Copy link
Member

bcoe commented Aug 22, 2019

@gajus actually, I went ahead and reopened this issue with a new title 👍

Can you figure out a reproduction that doesn't involve a command directory?

require('./')
  .env('ADM')
  .command('my-cool-command', 'description', (yargs) => {
  yargs
    .options({
      'cinema-data-scraper-sources-path': {
        demand: true,
        description: 'Path to the directory that includes the cinema data scraper sources.',
        type: 'string',
      },
      // [..]
    });
  }, () => {})
  .help()
  .wrap(80)
  .parse();

👆 the above works correctly for me.

@gajus
Copy link
Contributor

gajus commented Aug 22, 2019

I have tried a bunch of cases and I cannot replicate this without using command directory.

@gajus gajus closed this as completed Aug 22, 2019
@gajus gajus reopened this Aug 22, 2019
@gajus
Copy link
Contributor

gajus commented Aug 22, 2019

Will give it a try tomorrow again.

@gajus
Copy link
Contributor

gajus commented Aug 22, 2019

I cannot replicate it in a single file, but with commands it is easy:

// ./bin.js
require('yargs')
  .commandDir('commands')
  .parse();
// ./commands/foo.js
module.exports = {
  command: 'foo',
  desc: '',
  builder: () => {},
  handler: async () => {
    throw new Error('test');
  }
};
(node:23038) UnhandledPromiseRejectionWarning: TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Object.help (/Users/gajus/Documents/dev/temp3/node_modules/yargs/lib/usage.js:240:22)
    at Object.self.showHelp (/Users/gajus/Documents/dev/temp3/node_modules/yargs/lib/usage.js:432:15)
    at Object.Yargs.self.showHelp (/Users/gajus/Documents/dev/temp3/node_modules/yargs/yargs.js:790:11)
    at Object.fail (/Users/gajus/Documents/dev/temp3/node_modules/yargs/lib/usage.js:50:17)
    at /Users/gajus/Documents/dev/temp3/node_modules/yargs/lib/command.js:239:36
(node:23038) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:23038) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@bcoe
Copy link
Member

bcoe commented Aug 23, 2019

@gajus you're not using using TypeScript are you? I found a bug that throws similar errors to yours, that happens as a result of:

var __importStar = (this && this.__importStar) || function (mod) {
    if (mod && mod.__esModule) return mod;
    var result = {};
    if (mod != null) for (var k in mod) if (Object.hasOwnProperty.call(mod, k)) result[k] = mod[k];
    result["default"] = mod;
    return result;
};
const yargs = __importStar(require("yargs"));

The __importStar helper in TypeScript executes all the methods on yargs, which in turn seems to put the parser in a strange state. Any thoughts @mleguen? I'm wondering if this could be the root cause of some of the issues we're seeing.

@gajus
Copy link
Contributor

gajus commented Aug 23, 2019

I am not using TypeScript.

@gajus
Copy link
Contributor

gajus commented Aug 23, 2019

What is the problem with this test case?

#1144 (comment)

@bcoe
Copy link
Member

bcoe commented Aug 23, 2019

@gajus test case should be fine, haven't dug into it too much yet; was just wondering if something similar is being caused by your example, i.e., all the fields being walked on an yargs instance (will need to dig and see).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants