Skip to content

Conversation

@fmfmartins
Copy link
Member

@fmfmartins fmfmartins commented Apr 17, 2023

Motivation

I've been experimenting with more recent versions of yarn and PnP support for a new blockchain-toolkit library.

The way the uphold-scripts commands are set up rely on commander's default behaviour for sub-commands.

I believe the reason it was worked so far for the projects that use it as a dependency is because of the existence of the node_modules directory and the way node is able to resolve modules/binaries/scripts resorting to it.

Well, the idea behind PnP makes it so that a node_modules folder is no longer necessary and the resolving is all managed by yarn instead.

Because of that I think a bug has been found. According to the sub-commands the way we've currently set up the program would make it so that:

Commander will search the files in the directory of the entry script for a file with the name combination command-subcommand, like pm-install or pm-search in the example below.

However, instead of looking at the name of the program it's looking at the filename instead.

By adding the executableFile property to each command we can specify the executable for each command and stop relying on this implicit behaviour.

This shouldn't be a breaking change.

Examples

Before:

~/Projects/uphold/blockchain-toolkit feature/add-constellation-address-tools* 12s
❯ yarn changelog
/Users/filipe.martins/Projects/uphold/blockchain-toolkit/.yarn/cache/commander-npm-8.3.0-c0d18c66d5-0f82321821.zip/node_modules/commander/lib/command.js:1021
        throw new Error(executableMissing);
        ^

Error: 'index-changelog' does not exist
 - if 'changelog' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead
 - if the default executable name is not suitable, use the executableFile option to supply a custom name
    at ChildProcess.<anonymous> (/Users/filipe.martins/Projects/uphold/blockchain-toolkit/.yarn/cache/commander-npm-8.3.0-c0d18c66d5-0f82321821.zip/node_modules/commander/lib/command.js:1021:15)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess._handle.onexit (node:internal/child_process:289:12)
    at onErrorNT (node:internal/child_process:476:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Node.js v18.15.0

After:

~/Projects/uphold/blockchain-toolkit feature/add-constellation-address-tools*
❯ yarn changelog

No output but works as expected by creating a CHANGELOG.md file that contains the following:

# Changelog

## [0.0.0](https://github.com/uphold/blockchain-toolkit/releases/tag/v0.0.0) (2023-04-17)

@diogotorres97 diogotorres97 merged commit 74cd303 into master May 9, 2023
@diogotorres97 diogotorres97 deleted the enhancement/set-executablefile-property branch May 9, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants