Skip to content

Commit

Permalink
feat: stop using getters and setters for flags
Browse files Browse the repository at this point in the history
  • Loading branch information
mdonnalley committed Sep 25, 2023
1 parent 4d14780 commit 57794c3
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 97 deletions.
59 changes: 16 additions & 43 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ stdout.on('error', (err: any) => {
throw err
})

const jsonFlag = {
export const jsonFlag = {
json: boolean({
description: 'Format output as json.',
helpGroup: 'GLOBAL',
Expand Down Expand Up @@ -128,22 +128,7 @@ export abstract class Command {

protected static '_--' = false

protected static _enableJsonFlag = false

public static get enableJsonFlag(): boolean {
return this._enableJsonFlag
}

public static set enableJsonFlag(value: boolean) {
this._enableJsonFlag = value
if (value === true) {
this.baseFlags = jsonFlag
} else {
delete this.baseFlags?.json
this.flags = {} // force the flags setter to run
delete this.flags?.json
}
}
public static enableJsonFlag = false

public static get '--'(): boolean {
return Command['_--']
Expand Down Expand Up @@ -184,29 +169,10 @@ export abstract class Command {
return cmd._run<ReturnType<T['run']>>()
}

protected static _baseFlags: FlagInput

static get baseFlags(): FlagInput {
return this._baseFlags
}

static set baseFlags(flags: FlagInput) {
// eslint-disable-next-line prefer-object-spread
this._baseFlags = Object.assign({}, this.baseFlags, flags)
this.flags = {} // force the flags setter to run
}
public static baseFlags: FlagInput

/** A hash of flags for the command */
protected static _flags: FlagInput

public static get flags(): FlagInput {
return this._flags
}

public static set flags(flags: FlagInput) {
// eslint-disable-next-line prefer-object-spread
this._flags = Object.assign({}, this._flags ?? {}, this.baseFlags, flags)
}
public static flags: FlagInput

public id: string | undefined

Expand Down Expand Up @@ -352,12 +318,19 @@ export abstract class Command {
}
}

protected async parse<F extends FlagOutput, B extends FlagOutput, A extends ArgOutput>(options?: Input<F, B, A>, argv = this.argv): Promise<ParserOutput<F, B, A>> {
protected async parse<F extends FlagOutput, B extends FlagOutput, A extends ArgOutput>(
options?: Input<F, B, A>,
argv = this.argv,
): Promise<ParserOutput<F, B, A>> {
if (!options) options = this.ctor as Input<F, B, A>
const opts = {context: this, ...options}
// the spread operator doesn't work with getters so we have to manually add it here
opts.flags = options?.flags
opts.args = options?.args
const combinedFlags = {...options.baseFlags, ...options.flags}

const opts = {
context: this,
...options,
flags: (options.enableJsonFlag ? {...combinedFlags, ...jsonFlag} : combinedFlags) as FlagInput<F>,
}

const results = await Parser.parse<F, B, A>(argv, opts)
this.warnIfFlagDeprecated(results.flags ?? {})

Expand Down
41 changes: 36 additions & 5 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import {Hook, Hooks, PJSON, Topic} from '../interfaces'
import {Plugin as IPlugin, Options} from '../interfaces/plugin'
import {URL, fileURLToPath} from 'node:url'
import {arch, userInfo as osUserInfo, release, tmpdir, type} from 'node:os'
import {compact, ensureArgObject, getHomeDir, getPlatform, isProd, requireJson} from '../util'
import {compact, ensureArgObject, getHomeDir, getPlatform, isProd, pickBy, requireJson} from '../util'
import {join, sep} from 'node:path'

import {Command} from '../command'
import {Performance} from '../performance'
import PluginLoader from './plugin-loader'
import {boolean} from '../flags'
import {format} from 'node:util'
import {getHelpFlagAdditions} from '../help'
import {loadWithData} from '../module-loader'
Expand Down Expand Up @@ -855,10 +855,32 @@ const defaultArgToCached = async (arg: Arg<any>, respectNoCacheDefault: boolean)
}
}

export async function toCached(c: Command.Class, plugin?: IPlugin, respectNoCacheDefault = false): Promise<Command.Cached> {
export async function toCached(cmd: Command.Class, plugin?: IPlugin, respectNoCacheDefault = false): Promise<Command.Cached> {
const flags = {} as {[k: string]: Command.Flag.Cached}

for (const [name, flag] of Object.entries(c.flags || {})) {
// In order to collect static properties up the inheritance chain, we need to recursively
// access the prototypes until there's nothing left. This allows us to combine baseFlags
// and flags as well as add in the json flag if enableJsonFlag is enabled.
const mergePrototype = (result: Command.Class, cmd: Command.Class): Command.Class => {
const proto = Object.getPrototypeOf(cmd)
const filteredProto = pickBy(proto, v => v !== undefined) as Command.Class
return Object.keys(proto).length > 0 ? mergePrototype({...filteredProto, ...result} as Command.Class, proto) : result
}

const c = mergePrototype(cmd, cmd)

const cmdFlags = {
...(c.enableJsonFlag ? {
json: boolean({
description: 'Format output as json.',
helpGroup: 'GLOBAL',
}),
} : {}),
...c.flags,
...c.baseFlags,
} as typeof c['flags']

for (const [name, flag] of Object.entries(cmdFlags || {})) {
if (flag.type === 'boolean') {
flags[name] = {
name,
Expand Down Expand Up @@ -946,7 +968,16 @@ export async function toCached(c: Command.Class, plugin?: IPlugin, respectNoCach
}

// do not include these properties in manifest
const ignoreCommandProperties = ['plugin', '_flags', '_enableJsonFlag', '_globalFlags', '_baseFlags']
const ignoreCommandProperties = [
'plugin',
'_flags',
'_enableJsonFlag',
'_globalFlags',
'_baseFlags',
'baseFlags',
'_--',
'_base',
]
const stdKeys = Object.keys(stdProperties)
const keysToAdd = Object.keys(c).filter(property => ![...stdKeys, ...ignoreCommandProperties].includes(property))
const additionalProperties: Record<string, unknown> = {}
Expand Down
6 changes: 3 additions & 3 deletions src/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@ type NotArray<T> = T extends Array<any> ? never: T;

export function custom<T = string, P extends CustomOptions = CustomOptions>(
defaults: Partial<OptionFlag<T[], P>> & {
multiple: true
multiple: true;
} & (
{required: true} | {default: OptionFlag<T[], P>['default']}
),
): FlagDefinition<T, P, {multiple: true; requiredOrDefaulted: true}>

export function custom<T = string, P extends CustomOptions = CustomOptions>(
defaults: Partial<OptionFlag<T, P>> & {
defaults: Partial<OptionFlag<NotArray<T>, P>> & {
multiple?: false | undefined;
} & (
{required: true} | {default: OptionFlag<NotArray<T>, P>['default']}
),
): FlagDefinition<T, P, {multiple: false; requiredOrDefaulted: true}>

export function custom<T = string, P extends CustomOptions = CustomOptions>(
defaults: Partial<OptionFlag<T, P>> & {
defaults: Partial<OptionFlag<NotArray<T>, P>> & {
default?: OptionFlag<NotArray<T>, P>['default'] | undefined;
multiple?: false | undefined;
required?: false | undefined;
Expand Down
3 changes: 2 additions & 1 deletion src/interfaces/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export type BooleanFlag<T> = FlagProps & BooleanFlagProps & {
}

export type OptionFlag<T, P = CustomOptions> = FlagProps & OptionFlagProps & {
parse: FlagParser<T, string, P>
parse: FlagParser<T | undefined, string, P>
defaultHelp?: FlagDefaultHelp<T, P>;
input: string[];
default?: FlagDefault<T | undefined, P>;
Expand Down Expand Up @@ -361,6 +361,7 @@ export type Flag<T> = BooleanFlag<T> | OptionFlag<T>
export type Input<TFlags extends FlagOutput, BFlags extends FlagOutput, AFlags extends ArgOutput> = {
flags?: FlagInput<TFlags>;
baseFlags?: FlagInput<BFlags>;
enableJsonFlag?: true | false;
args?: ArgInput<AFlags>;
strict?: boolean;
context?: ParserContext;
Expand Down
147 changes: 102 additions & 45 deletions test/command/command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ describe('command', () => {
static description = 'test command'
static aliases = ['alias1', 'alias2']
static hidden = true
// @ts-ignore
static flags = {
flaga: Flags.boolean(),
flagb: Flags.string({
Expand All @@ -98,8 +97,7 @@ describe('command', () => {
required: false,
description: 'flagc desc',
options: ['a', 'b'],
// @ts-expect-error: context is any
default: async context => context.options.min + 1,
default: async context => (context.options.min ?? 1) + 1,
}),
}

Expand Down Expand Up @@ -133,6 +131,8 @@ describe('command', () => {
deprecateAliases: undefined,
summary: undefined,
strict: true,
enableJsonFlag: false,
hasDynamicHelp: false,
flags: {
flaga: {
aliases: undefined,
Expand Down Expand Up @@ -224,51 +224,108 @@ describe('command', () => {
.it('converts to cached with everything set')

fancy
// .skip()
.do(async () => {
// const c = class extends Command {
// }.convertToCached()
// expect(await c.load()).to.have.property('run')
// delete c.load
// expect(c).to.deep.equal({
// _base: `@oclif/command@${pjson.version}`,
// id: undefined,
// type: undefined,
// hidden: undefined,
// pluginName: undefined,
// description: 'test command',
// aliases: [],
// title: undefined,
// usage: undefined,
// flags: {},
// args: [],
// })
})
class Base extends Command {
public static enableJsonFlag = true
public static baseFlags = {
parentFlag: Flags.boolean(),
}
}

class Child extends Base {
static flags = {
childFlag: Flags.boolean(),
}
}

.it('adds plugin name')
const cached = await toCached(Child, undefined, false)

fancy
// .skip()
// .do(async () => {
// const c = class extends Command {
// }.convertToCached({pluginName: 'myplugin'})
// expect(await c.load()).to.have.property('run')
// delete c.load
// expect(c).to.deep.equal({
// _base: `@oclif/command@${pjson.version}`,
// type: undefined,
// id: undefined,
// hidden: undefined,
// pluginName: 'myplugin',
// description: 'test command',
// aliases: [],
// title: undefined,
// usage: undefined,
// flags: {},
// args: [],
// })
// })
.it('converts to cached with nothing set')
expect(cached).to.deep.equal({
id: 'command',
summary: undefined,
description: 'test command',
strict: true,
usage: undefined,
pluginName: undefined,
pluginAlias: undefined,
pluginType: undefined,
hidden: undefined,
state: undefined,
aliases: [],
examples: undefined,
deprecationOptions: undefined,
deprecateAliases: undefined,
flags: {
json: {
name: 'json',
type: 'boolean',
char: undefined,
summary: undefined,
description: 'Format output as json.',
hidden: undefined,
required: undefined,
helpLabel: undefined,
helpGroup: 'GLOBAL',
allowNo: false,
dependsOn: undefined,
relationships: undefined,
exclusive: undefined,
deprecated: undefined,
deprecateAliases: undefined,
aliases: undefined,
charAliases: undefined,
delimiter: undefined,
noCacheDefault: undefined,
},
childFlag: {
name: 'childFlag',
type: 'boolean',
char: undefined,
summary: undefined,
description: undefined,
hidden: undefined,
required: undefined,
helpLabel: undefined,
helpGroup: undefined,
allowNo: false,
dependsOn: undefined,
relationships: undefined,
exclusive: undefined,
deprecated: undefined,
deprecateAliases: undefined,
aliases: undefined,
charAliases: undefined,
delimiter: undefined,
noCacheDefault: undefined,
},
parentFlag: {
name: 'parentFlag',
type: 'boolean',
char: undefined,
summary: undefined,
description: undefined,
hidden: undefined,
required: undefined,
helpLabel: undefined,
helpGroup: undefined,
allowNo: false,
dependsOn: undefined,
relationships: undefined,
exclusive: undefined,
deprecated: undefined,
deprecateAliases: undefined,
aliases: undefined,
charAliases: undefined,
delimiter: undefined,
noCacheDefault: undefined,
},
},
args: {},
hasDynamicHelp: false,
enableJsonFlag: true,
})
})
.it('converts to cached with multiple Command classes in inheritance chain')
})

describe('parse', () => {
Expand Down

0 comments on commit 57794c3

Please sign in to comment.