Support async plugins / extensions

[orta]

Code I'd like to write:

import { ghauth } from "../util/promised_ghauth"
import { Context } from "../context"

const setGithubCredentials = async (context: Context) => {
  context.githubAuth = await ghauth()
}

module.exports = setGithubCredentials

and

import { getSettings } from "../util/settings"
import { Context } from "../context"

const setSettings = async (context: Context) => {
  context.settings = await getSettings()
}

module.exports = setSettings

The GH Auth prompt does a bunch of pre-requisite work, it's feasible that I can add a function to handle it per-command but it's basically something I want on the first time user flow anyway.

I took a quick look at this and it seems like it'd add async code everywhere so opted to not PR right now and to move it to a function my command can handle as I only have one ;)

Opened so people can decide if this is something you want

[jamonholmgren]

This is an interesting use case. I'm not against it in principle, but I think that we'd want to avoid await in each extension, as it could slow down setup when the result may not even be used in the given command.

Right now, you could do something like this:

import { ghauth } from "../util/promised_ghauth"
import { Context } from "../context"

const setGithubCredentials = (context: Context) => {
  context.githubAuth = ghauth
}

module.exports = setGithubCredentials

... and in your command, do this:

const githubAuth = await context.githubAuth()
// do whatever with githubAuth now

If it's not needed, then you never kick off the async function.

I suppose you could also attach the promise and kick it off immediately in the extension.

import { ghauth } from "../util/promised_ghauth"
import { Context } from "../context"

const setGithubCredentials = (context: Context) => {
  context.githubAuthPromise = ghauth()
}

module.exports = setGithubCredentials

const githubAuth = await context.githubAuthPromise

This will kick things off but not await until you're ready to use it.

The problem with this, of course, is that you'd need to make sure you always catch that promise, or Node will complain about it.

[orta]

Yeah the promise might probably be the best option TBH 👍

[jamonholmgren]

I'm going to close this issue, given we have a reasonable pattern to follow above. Happy to restart the discussion if anyone has strong feelings about it.

[RichiCoder1]

I actually do. Basically:

• We want to ship a default config out of band from local nat accessible services. It be nice to either load this or warn the user their current config is out of date before a command runs.
• General outdated checks.

There was another use cases I though of, but forgot.

It looks like it'd be a simple change, would you take a PR?

[jamonholmgren]

@RichiCoder1 I'm not entirely following, can you explain how this would affect things both internally in Gluegun and for the CLI author?

[RichiCoder1]

I'm realizing that my use case may be slightly different than the authors. But the idea would be like this: https://github.com/infinitered/gluegun/pull/449

Basically so I can have an extension like: companySettings.ts which has

import { getSettingsAsync } from "../my/helper";

function async setupPluginRemoteConfig(toolbox: MyCompanyToolbox) {
    const settings = await getSettingsAsync();
    toolbox.plugin = ((plugin) => {
        plugin.settings = settings;
    })(toolbox.plugin || {}):    
}
export default setupPluginRemoteConfig;

Edit: or something like updateCheck.ts

import { isOutdated } from 'is-outdated';
const  = require('./package.json');
 
function async checkForNewVersion(toolbox) {
    const { version, outdated } = await isOutdated(pkg.name, pkg.version);
    const { print: { warn } } = toolbox;
    if (outdated) {
        warn('The latest version of this app is %s', res.version);
        warn(`Please updated it with: npm update -g ${pkg.name}`);
    }
}
export default checkForNewVersion;

Though there's an argument to be made for the latter being in gluegun itself.

[jamonholmgren]

I really like the idea of is-outdated being built in to Gluegun. Perhaps on toolbox.meta.outdated() as an async function?

Regarding your use case, I can see how that would be helpful. Since it won't break backwards compatibility, I'm open to it. ref #449

[RichiCoder1]

I really like the idea of is-outdated being built in to Gluegun. Perhaps on toolbox.meta.outdated() as an async function?

Indeed! I like it. Though it might also be good to have it on builder too. Something like:

// ready
const { build } = require('gluegun')
const package = require('../package.json')

// aim
const movieCLI = build('movie')
  .src(`${__dirname}/core-plugins`)
  .plugins('node_modules', { matching: 'movie-*' })
  .help()
  .version()
  .checkForUpdates(package.name, package.version)
  .defaultCommand()
  .create()

// fire!
movieCLI.run()

Probably worth it's own issue.