4
\$\begingroup\$

I'm writing a command line app in node.js. At this stage, the app does the following:

  1. Get the version from package.json
  2. Update the package.json with a new version depending on the command line argument
  3. Commit the changes
  4. Tag the commit
  5. Push commits and tags to the remote

Some comments:

  • I'm really new to async/await and promises so I may be using those incorrectly in my code.
  • A lot of my code is wrapped in try-catch blocks which I understand is not supposed to be the case with async/await or Promises. I'm not sure how to fix this.
  • There's a lot of duplication with the spinner initialisation code. I am aware the ora has an ora.promise method as well. I couldn't get this to work.
  • Another example of duplication is with the process.exit(1) line within each catch block. Is this really needed or are there better ways to achieve this?
  • Do I really need to use the async IIFE at the end or is there a better way to do this?
  • I would also like feedback on whether I've divided my code into functions adequately.
  • I haven't added much error checking yet. For example, later, I'd like to check if a package.json exists, and if it doesn't, get the current version from git tags. How can I structure my code in such a way that I can easily add such functionality in the future?

Any other feedback is also welcome. If you can address even some of the above, I would appreciate it.

Usage:

ship <major|premajor|minor|preminor|patch|prepatch|prerelease>

(I named the script index.js, and created a symbolic link ship to it in /usr/local/bin)

#!/usr/bin/env node
'use strict';

const fs = require('fs');
const semver = require('semver');
const readPkg = require('read-pkg');
const writePkg = require('write-pkg');
const git = require('simple-git/promise')(process.cwd());

const ora = require('ora');
const chalk = require('chalk');

const env = process.env;

const validArgs = [
    'major', 'premajor', 'minor', 'preminor', 'patch', 'prepatch', 'prerelease'
];

const argv = require('yargs').argv;
const arg = argv._[0];

let spinner;

if (arg === undefined) {
    console.error(chalk.red(`Missing argument.`));
    console.error(chalk.red(`Should be one of ${validArgs.join(', ')}.`));
    process.exit(1);
}

if (!validArgs.includes(arg)) {
    console.error(chalk.red(`Invalid argument ${arg}`));
    process.exit(1);
}

async function nextVersion() {
    let current;

    try {
        const data = await readPkg();
        current = data.version;
    } catch(err) {
        process.exit(1);
        // todo: get version from git tags
    }

    return semver.inc(current, arg);
}

async function updatePkg(version) {
    spinner = ora('Writing package.json').start();

    try {
        const data = await readPkg();
        delete data._id;
        data.version = version;
        await writePkg(data);
        spinner.succeed();
    } catch (err) {
        spinner.fail();
        process.exit(1);
    }
}

async function commit(msg) {
    spinner = ora('Commiting changes').start();

    try {
        await git.add('package.json');
        await git.commit(msg);
        spinner.succeed();
    } catch (err) {
        spinner.fail();
        process.exit(1);
    }
}

async function makeTags(name) {
    spinner = ora('Adding tag').start();

    try {
        await git.addTag(name);
        spinner.succeed();
    } catch (err) {
        spinner.fail();
        process.exit(1);
    }
}

async function push() {
    spinner = ora('Pushing changes').start();

    try {
        // both of these may run together
        await git.push();
        await git.pushTags();
        spinner.succeed();
    } catch (err) {
        spinner.fail();
        process.exit(1);
    }
}

(async () => {
    const next = await nextVersion();

    console.log(); // gap between input and spinners

    await updatePkg(next);
    await commit(next);
    await makeTags(next);
    await push();
})();
\$\endgroup\$
3
\$\begingroup\$

(Disclaimer: I'm not a JavaScript expert and I don't have much experience with async/await either, so take my advices with a grain of salt. I hope you will find it helpful anyway.)

Running async calls in parallel

If you assume these will run in parallel, they won't:

// both of these may run together
await git.push();
await git.pushTags();

To make them run in parallel, you could use Promise.all:

await Promise.all([git.push(), git.pushTags()]);

Or save the result of the calls without await, and await on the results:

const pushResult = git.push();
const pushTagsResult = git.pushTags();
await pushResult;
await pushTagsResult;

Not too much async and spinners?

Most of the operations are fast, such as updating the file, git add, and git commit, and I don't see a good reason to make them run in parallel and with a progress indicator.

The only operation that may be slow and where the progress indicator makes sense is the git push.

Since the main logic is not packaged as reusable code that can be imported and called from another module, wrapping it in async (in the IIFE at the end) seems unnecessary too. If the updating of the package was separated from the command line parsing, then it could make sense to make it possible to run async, if you had a use case for that. Without a use case, I think it's over-engineering.

Addressing your questions

A lot of my code is wrapped in try-catch blocks which I understand is not supposed to be the case with async/await or Promises. I'm not sure how to fix this.

I don't see anything wrong with your try-catch blocks (but see the disclaimer at the top).

There's a lot of duplication with the spinner initialisation code. I am aware the ora has an ora.promise method as well. I couldn't get this to work.

As mentioned earlier, I think the spinners are overkill. However, even if you remove them from most places, much duplication will still remain.

I don't think you need the granularity of catching exception at every step. You could wrap those steps in a single try-catch block, and that will reduce the duplication and boilerplate code.

I haven't added much error checking yet. For example, later, I'd like to check if a package.json exists, and if it doesn't, get the current version from git tags. How can I structure my code in such a way that I can easily add such functionality in the future?

The way you did so far seems fine to me. If later you want to get tags from Git, you can add a function for that, and call it from nextVersion as needed.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.