Properly merge two objects

[late sinew]

I have a library where a default config is defined as an object and assigned to CONFIG. However, a user of the library can pass-in a customized config. We merge these, of course. I ran into a bump along the way, as I discovered that assigned objects create a reference to the object; so when I pass CONFIG into the merge function I need to ensure I don't alter the original CONFIG by way of reference.

This is how I resolved the issue, but wanted some feedback on if there is a better way. I created the deepCopy function for the purposes of documentation - as I'd otherwise have no intuition that I'm 'deep copying' an object using the stringify/parse code.

const deepCopy = (object: {}): any => {
  return JSON.parse(JSON.stringify(object))
}

let initial_merge_call = true

export const mergeObjects = ({ current, updates }: { current: KeyStringObjectAny, updates: KeyStringObjectAny }): any => {
  if (!current || !updates)
    throw new Error("Both 'current' and 'updates' must be passed-in to merge()")
  if (typeof current !== 'object' || typeof updates !== 'object')
    throw new Error("Both 'current' and 'updates' must be passed-in as objects to merge()")

  /**
   * We need to make a deep copy of `current`, otherwise we end up altering the original `CONFIG` because
   * `current` is a reference to it; but we can't deep copy recursive merges of objects as that would break
   * a needed reference to `merged`.
   */
  let merged = initial_merge_call ? deepCopy(current) : current

  for (let key of Object.keys(updates)) {
    if (typeof updates[key] !== 'object') {
      merged[key] = updates[key]
    } else {
      /* key is an object, run merge on it */
      initial_merge_call = false
      mergeObjects({ current: merged[key], updates: updates[key] })
    }
  }

  initial_merge_call = true
  return merged
}

[sacred lotus]

would lodash clone deep help you achieve what you need?

https://lodash.com/docs/#cloneDeep

[limber cairn]

Or perhaps Object.assign?

[late sinew]

In this case, I'd prefer something local as opposed to adding a dependency.

[mighty crow]

Solutions like lodash are open source, you can see how they did it also to find corner cases. If this is mission critical logic its best to look at a proven solution to ensure nothing slips through the cracks.

[worldly dust]

how does the spread operator behave in this scenario?

like if you spread the default config into an object, then spread the custom pieces over that.

I can't remember if that would overwrite keys as needed or not.

that would be a shallow merge though, if you have nested keys that need to merge that wouldn't work

[boreal mountain]

The use of a global flag can lead to unexpected behaviors if mergeObjects is called concurrently or recursively from different parts of your codebase. Instead of using a global flag, you can use function arguments or create a helper function to manage the recursive behavior.

const deepCopy = (object: any): any => {
  return JSON.parse(JSON.stringify(object));
}

const mergeHelper = (current: any, updates: any): any => {
  if (!current || !updates) {
    throw new Error("Both 'current' and 'updates' must be passed-in to merge()");
  }
  if (typeof current !== 'object' || typeof updates !== 'object' || Array.isArray(current) || Array.isArray(updates)) {
    throw new Error("Both 'current' and 'updates' must be passed-in as non-array objects to merge()");
  }

  let merged = { ...current };
  
  for (let key of Object.keys(updates)) {
    if (typeof updates[key] !== 'object' || updates[key] === null) {
      merged[key] = updates[key];
    } else {
      merged[key] = mergeHelper(merged[key] || {}, updates[key]);
    }
  }

  return merged;
}

export const mergeObjects = ({ current, updates }: { current: any, updates: any }): any => {
  return mergeHelper(deepCopy(current), updates);
}

handy unit tests

import { mergeObjects } from './yourFilePath';  // Adjust the import path accordingly
import assert from 'assert';

// Utility function to wrap each test and provide error handling
function runTest(testName: string, testFunc: () => void) {
  try {
    testFunc();
    console.log(`✅ ${testName} passed`);
  } catch (error) {
    console.error(`❌ ${testName} failed: ${error.message}`);
  }
}

runTest('should merge two flat objects', () => {
  const current = { a: 1, b: 2 };
  const updates = { b: 3, c: 4 };
  const result = mergeObjects({ current, updates });
  assert.deepStrictEqual(result, { a: 1, b: 3, c: 4 });
});

runTest('should merge nested objects', () => {
  const current = { a: 1, b: { x: 10, y: 20 } };
  const updates = { b: { y: 25, z: 30 }, c: 4 };
  const result = mergeObjects({ current, updates });
  assert.deepStrictEqual(result, { a: 1, b: { x: 10, y: 25, z: 30 }, c: 4 });
});

runTest('should handle non-object types', () => {
  const current = { a: 1, b: 'string' };
  const updates = { b: 'newString', c: true };
  const result = mergeObjects({ current, updates });
  assert.deepStrictEqual(result, { a: 1, b: 'newString', c: true });
});

runTest('should not mutate the original objects', () => {
  const current = { a: 1, b: { x: 10 } };
  const updates = { b: { x: 15 }, c: 5 };
  mergeObjects({ current, updates });
  assert.deepStrictEqual(current, { a: 1, b: { x: 10 } });
  assert.deepStrictEqual(updates, { b: { x: 15 }, c: 5 });
});

runTest('should throw an error for missing parameters', () => {
  assert.throws(() => mergeObjects({ current: { a: 1 }, updates: undefined }));
  assert.throws(() => mergeObjects({ current: undefined, updates: { b: 2 } }));
});

runTest('should throw an error for non-object parameters', () => {
  assert.throws(() => mergeObjects({ current: { a: 1 }, updates: 'string' }));
  assert.throws(() => mergeObjects({ current: 'string', updates: { b: 2 } }));
  assert.throws(() => mergeObjects({ current: [1, 2], updates: { b: 2 } }));
  assert.throws(() => mergeObjects({ current: { a: 1 }, updates: [1, 2] }));
});

[late sinew]

This didn't work for nested objects, but it put me onto structuredClone(), which I used to replace deepCopy(), so thank you!

[late sinew]

Thanks Paul!! I didn't even think about the global causing an issue. I dropped it and mimicked the two merge functions. Thanks for the array checks as well - again, didn't think about the fact that they're objects.

[boreal mountain]

JS is wild, everything is an object, and you gotta lean into the event loop. If that makes no sense, lemme know.

[late sinew]

I forget most of this video at this point; prolly ought to re-watch. 🔥 https://youtu.be/8aGhZQkoFbQ?si=I8F-NwQ6cqkZigYe

[boreal mountain]

that's the one, when you're facing challenges with memory integrity like cacheing and memoization, it's a good reminder to know you can't outrun the event loop.

[late sinew]

Thanks @boreal mountain @limber cairn @worldly dust @mighty crow @sacred lotus
You're anonymously in the git history lol
13aa (HEAD -> v2.0.0-next) fix: rework merging. thanks LBT community!

[worldly dust]

I did nothing useful 😂

[late sinew]

You engaged, and that's important for the community!

[boreal mountain]

untrue, your comments got me intrigued into the spread operator :p

[worldly dust]

you get a chance to test that out? I haven't yet. Other work that isn't code 😭

[boreal mountain]

https://lodash.com/docs/#merge this is the abstraction

[late sinew]

Spread works in the context of his code. It did not work as a drop-in with my original function.

[worldly dust]

yeah spread would work for first level but wouldn't recurse. Thinking back I think I've use spread like this for entity objects when I need to update one or two fields though, so I'm pretty sure spreading the same keys into one object means the latter spread wins.

you know, what, browser is a JS repl let's check

yep, spreading merges and overwrites keys

[late sinew]

Yeah, I played around with Object.assign() as well and it overwrites the entire nested object; so any keys the original had, that the update didn't, were wiped out.

For reference, I tried this, because it seemed like the best chance of it working: let merged = Object.assign({}, current, updates) - it did not.