From 9fdb4637ead68df14517cd56a2d55676eccb08bd Mon Sep 17 00:00:00 2001
From: Mike Donnalley <mdonnalley@salesforce.com>
Date: Thu, 3 Oct 2024 22:00:17 -0600
Subject: [PATCH] fix: correctly merge command class prototypes (#765)

Co-authored-by: Steve Hetzel <shetzel@salesforce.com>
---
 src/commands/commands.ts       | 24 ++++++++++--------------
 test/commands/commands.test.ts |  9 ++++++++-
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/commands/commands.ts b/src/commands/commands.ts
index 724d8453..a6cfadce 100644
--- a/src/commands/commands.ts
+++ b/src/commands/commands.ts
@@ -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.'
 
@@ -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
diff --git a/test/commands/commands.test.ts b/test/commands/commands.test.ts
index c093329b..c125e8f8 100644
--- a/test/commands/commands.test.ts
+++ b/test/commands/commands.test.ts
@@ -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'
@@ -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
   }
@@ -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')