fix: Added isMergeableObject config to deepmerge (#1002)

* fix: Added isMergeableObject config to deepmerge * chore: add isMergeableObject test * fix: add ts-ignore for tests * test: complex vs plain merge --------- Co-authored-by: Andrew Gerard <[email protected]>
godaddy · Dec 20, 2024 · 357fe7c · 357fe7c
1 parent 63cdd2d
commit 357fe7c
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 30 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/gasket-utils/lib/config.js b/packages/gasket-utils/lib/config.js
@@ -1,20 +1,20 @@
 const deepmerge = require('deepmerge');
 // @ts-ignore - diagnostics lib does not have a types declaration file
 const debug = require('diagnostics')('gasket:utils');
+// @ts-ignore - 'isPlainObject' is not imported by using a default import
+const { isPlainObject } = require('is-plain-object');
 
 /**
  * Normalize the config by applying any overrides for environments, commands,
  * or local-only config file.
  * @type {import('./config').applyConfigOverrides}
  */
-function applyConfigOverrides(
-  config,
-  { env = '', commandId }
-) {
+function applyConfigOverrides(config, { env = '', commandId }) {
   // @ts-ignore - merged config definitions
   return deepmerge.all(
-  // @ts-ignore - partial config definitions
-    [...getPotentialConfigs(config, { env, commandId })].reverse()
+    // @ts-ignore - partial config definitions
+    [...getPotentialConfigs(config, { env, commandId })].reverse(),
+    { isMergeableObject: isPlainObject }
   );
 }
 
@@ -78,9 +78,7 @@ function *getDevOverrides(isLocalEnv, environments) {
   // development environment
   const devEnv = isLocalEnv && (environments.development || environments.dev);
   if (devEnv) {
-    debug(
-      'Including dev/development override due to local environment inheritance'
-    );
+    debug('Including dev/development override due to local environment inheritance');
     yield devEnv;
   }
 }

diff --git a/packages/gasket-utils/lib/index.d.ts b/packages/gasket-utils/lib/index.d.ts
@@ -64,7 +64,12 @@ export interface Signal {
  * Options can be passed to the underlying spawn. An additional `signal` option
  * can be passed to use AbortController, allowing processes to be killed when no
  * longer needed.
- *
+ * @param cmd
+ * @param argv
+ * @param options
+ * @param options.signal
+ * @param options.cwd
+ * @param debug
  * @example
  * const { runShellCommand } = require('@gasket/utils');
  *
@@ -132,6 +137,8 @@ declare module '@gasket/utils' {
    * Executes the appropriate npm binary with the verbatim `argv` and
    * `spawnWith` options provided. Passes appropriate debug flag for
    * npm based on process.env.
+   * @param argv
+   * @param spawnWith
    */
   function PackageManager_spawnNpm(
     /** Precise CLI arguments to pass to `npm`. */
@@ -144,6 +151,8 @@ declare module '@gasket/utils' {
    * Executes the appropriate yarn binary with the verbatim `argv` and
    * `spawnWith` options provided. Passes appropriate debug flag for
    * npm based on process.env.
+   * @param argv
+   * @param spawnWith
    */
   function PackageManager_spawnYarn(
     /** Precise CLI arguments to pass to `npm`. */
@@ -174,13 +183,7 @@ declare module '@gasket/utils' {
     args: string[]
   ): Promise<{ data: any; stdout: string }>;
 
-  export function warnIfOutdated(
-    pkgName: string,
-    currentVersion: string
-  ): MaybeAsync<void>;
+  export function warnIfOutdated(pkgName: string, currentVersion: string): MaybeAsync<void>;
 }
 
-export function getPackageLatestVersion(
-  pkgName: string,
-  options?: object
-): Promise<string>;
+export function getPackageLatestVersion(pkgName: string, options?: object): Promise<string>;
diff --git a/packages/gasket-utils/package.json b/packages/gasket-utils/package.json
@@ -48,6 +48,7 @@
     "concat-stream": "^2.0.0",
     "deepmerge": "^4.3.1",
     "diagnostics": "^2.0.2",
+    "is-plain-object": "^5.0.0",
     "semver": "^7.6.3"
   },
   "devDependencies": {

diff --git a/packages/gasket-utils/test/config.test.js b/packages/gasket-utils/test/config.test.js
@@ -1,3 +1,4 @@
+/* eslint-disable jest/no-conditional-expect */
 const { applyConfigOverrides } = require('../lib/config');
 
 describe('applyConfigOverrides', () => {
@@ -191,8 +192,41 @@ describe('applyConfigOverrides', () => {
     });
   });
 
-  describe('commands', function () {
+  it('copies (not merge) non-plain object when merging env', () => {
+    class ComplexObject {
+      isComplex = true;
+      constructor(value) {
+        this.value = value;
+      }
+      complexMethod() {
+        return true;
+      }
+    }
+
+    mockConfig.plain = { value: 'init' };
+    mockConfig.complex = new ComplexObject('init');
+    mockConfig.environments = {
+      dev: {
+        plain: { value: 'override' },
+        complex: new ComplexObject('override')
+      }
+    };
 
+    results = applyConfigOverrides(mockConfig, mockContext);
+
+    // complex should copy
+    expect(results.complex).toBe(mockConfig.environments.dev.complex);
+    expect(results.complex).toHaveProperty('value', 'override');
+    expect(results.complex).toHaveProperty('complexMethod', expect.any(Function));
+
+    // plain should merge (new object but equal)
+    expect(results.plain).not.toBe(mockConfig.environments.dev.plain);
+    expect(results.plain).toEqual(mockConfig.environments.dev.plain);
+    expect(results.plain).toHaveProperty('value', 'override');
+
+  });
+
+  describe('commands', function () {
     it('deep merges properties from matching command id', () => {
       mockConfig.commands = {
         start: {
@@ -247,6 +281,6 @@ describe('applyConfigOverrides', () => {
           }
         }
       });
-    })
+    });
   });
 });