Skip to content

Commit

Permalink
fix: correctly merge command class prototypes (#765)
Browse files Browse the repository at this point in the history
Co-authored-by: Steve Hetzel <[email protected]>
  • Loading branch information
mdonnalley and shetzel authored Oct 4, 2024
1 parent 85a2707 commit 9fdb463
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
24 changes: 10 additions & 14 deletions src/commands/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ function determineHeaders(columns: Column[] | undefined, extended: boolean | und
return [columnConfigs.id, columnConfigs.summary]
}

// In order to collect static properties up the inheritance chain, we need to recursively access the prototypes until there's nothing left
function mergePrototype(result: Command.Class, command: Command.Class): Command.Class {
const proto = Object.getPrototypeOf(command)
const filteredProto = _.pickBy(proto, (v) => v !== undefined) as Command.Class
return Object.keys(proto).length > 0 ? mergePrototype({...filteredProto, ...result} as Command.Class, proto) : result
}

export default class Commands extends Command {
static description = 'List all <%= config.bin %> commands.'

Expand Down Expand Up @@ -146,26 +153,15 @@ export default class Commands extends Command {
const json = _.uniqBy(
await Promise.all(
commands.map(async (cmd) => {
let commandClass: Command.Class | undefined
let commandClass: Command.Class
try {
commandClass = await cmd.load()
} catch (error) {
this.debug(error)
return cmd
}

const obj = {...cmd, ...commandClass}

// Load all properties on all extending classes.
while (commandClass !== undefined) {
commandClass = Object.getPrototypeOf(commandClass) ?? undefined
// ES2022 will return all unset static properties on the prototype as undefined. This is different from ES2021
// which only returns the static properties that are set by defaults. In order to prevent
// Object.assign from overwriting the properties on the object, we need to filter out the undefined values.
Object.assign(
obj,
_.pickBy(commandClass, (v) => v !== undefined),
)
}
const obj = {...mergePrototype(commandClass, commandClass), ...cmd}

// The plugin property on the loaded class contains a LOT of information including all the commands again. Remove it.
delete obj.plugin
Expand Down
9 changes: 8 additions & 1 deletion test/commands/commands.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Command} from '@oclif/core'
import {Args, Command} from '@oclif/core'
import {runCommand} from '@oclif/test'
import {expect} from 'chai'
import sinon from 'sinon'
Expand All @@ -16,9 +16,14 @@ obj.circular = obj

class AnotherTestCommand extends TestCommand {
public static anotherCustomProperty = [5, obj]
public static args = {
arg1: Args.string(),
}

public static circularObj = obj

public static enableJsonFlag = true

public async run() {
// Do nothing
}
Expand Down Expand Up @@ -104,6 +109,8 @@ describe('commands', () => {
expect(commands[0].testCustomProperty).to.equal('test')
expect(commands[0].anotherCustomProperty[0]).to.equal(5)
expect(commands[0].anotherCustomProperty[1]).to.deep.equal({foo: 'bar'})
expect(commands[0].args).to.have.property('arg1')
expect(commands[0].enableJsonFlag).to.be.true
expect(commands[0]).to.not.have.property('circularObj')
expect(commands[1].id).to.equal('topic:subtopic:command')
expect(commands[1].testCustomProperty).to.equal('test')
Expand Down

0 comments on commit 9fdb463

Please sign in to comment.