positional argument value is undefined in v1.1.0

[adam-lynch]

@vadimdemedes @karaggeorge 1.1.0 is broken unfortunately.

When I run todesktop build ., todesktop build ./, etc. the argument is undefined. I see the usual prop-type warning that required property X is missing and then the command fails with an error (I pass the positional argument value to path.resolve).

It works fine when I go back to 1.0.3.

[adam-lynch]

My command looks like this:

// ...

/// Build an Electron app with native installers, code signing baked-in, etc. but without releasing it (existing users won't get an auto-update). For quicker builds, you can append `--code-sign=false` to disable code-signing and notarization.
const BuildCommand = ({ codeSign: shouldCodeSign = true, projectPath }) => {
  // ...
};

BuildCommand.propTypes = {
  /// The path to the root of your project (where todesktop.json is)
  projectPath: PropTypes.string.isRequired,
  /// Whether or not code-signing and notarization should be performed. Disable this for quicker builds
  codeSign: PropTypes.bool,
};
BuildCommand.positionalArgs = ["projectPath"];
export default BuildCommand;

[karaggeorge]

I'm investigating to see what happened

[karaggeorge]

Hmm, I can't reproduce. When I run (my command is named hello):

• my-cli hello ., I get this in the props:

{
  _: [ 'hello' ],
  codeSign: true,
  projectPath: '.',
  '$0': '/usr/local/bin/my-cli',
  inputArgs: [ 'hello' ]
}

• my-cli hello ./

{
  _: [ 'hello' ],
  codeSign: undefined,
  projectPath: './',
  '$0': '/usr/local/bin/my-cli',
  inputArgs: [ 'hello' ]
}

So both cases seem to be working for me. And if I add --code-sign to either I also get the flag correctly

I just copied the command from above, and console.log the props in the component

[adam-lynch]

I'm struggling to reproduce it now too. I tried to create a simple example. But there definitely is an issue. I have two npm releases of my CLI (which are identical except for the pastel version) and only one has this issue.

I compared the contents of node_modules after installing the two versions. My CLI's contents are pretty much unchanged. We require the CLI's own package.json in the code. So that line is different between the two versions.

The only other changes I see are in node_modules/pastel.

Got any ideas of what I could try to troubleshoot this?

[adam-lynch]

I added some logging to boot.js. Is it expected that command.positionalArgs is empty and my positional argument is in command.args? command.args contains this:

[
    {
      key: 'projectPath',
      type: 'string',
      description: 'The path to the root of your project (where todesktop.json is)',
      isRequired: true,
      aliases: [],
      positional: true
    },
    {
      key: 'codeSign',
      type: 'boolean',
      description: 'Whether or not code-signing and notarization should be performed. Disable this for quicker builds',
      isRequired: false,
      aliases: [],
      positional: false
    }
  ],

This seems to be related to the issue because if I make this change, my CLI works as expected:

-           positionalArgs = [],
+           positionalArgs = ["projectPath"],

(https://github.com/vadimdemedes/pastel/blob/master/boot.js#L35)

[karaggeorge]

Can you try to rebuild? Pastel didn't used to include positionalArgs array in the generated commands.json but after 1.1.0 it does. It looks like that is missing, even though it has correctly flagged your projectPath as positional: true. Can you check the commands.json file in your build directory and see if the commands have the positionalArgs array?

[karaggeorge]

@adam-lynch Can you also try clearing caches before rebuilding? It might not be generating the files again because there haven't been any code changes. Try removing node_modules/.cache

[adam-lynch]

Can you try to rebuild?

I did and can't reproduce as I said above.

Pastel didn't used to include positionalArgs array in the generated commands.json but after 1.1.0 it does. It looks like that is missing, even though it has correctly flagged your projectPath as positional: true.

Couldn't that be it then? I built my CLI with 1.0.3, published to npm with pastel@^1.0.3 as a dependency. A user installed it after 1.1.0 was published, so it used 1.1.0 but the positionalArgs array is empty. So the positionalArgs array handling needs to be backwards-compatible & tolerant a commands.json that was built with < 1.1.0.

[karaggeorge]

Yeah that's probably it. I can open a PR and add some backwards compatibility. Expect it later today.

Honestly I didn't think about backwards compatibility, but I also didn't know it would be released as a minor version, so I didn't think it would be an issue.

[karaggeorge]

@adam-lynch see #33. Not sure if there's a way for you to test using that, maybe npm link locally, but I tested it with a commands.json missing the positionalArgs array and it fixed it before running

[vadimdemedes]

Fix released in 1.1.1.