refactor(Table): prefer Object.assign where applicable

[jelly]

mergeWith is called without a third function argument making it a plain merge which can be implemented with Object.assign as this function should always receive objects.

What: Closes #

Additional issues:

Summary by CodeRabbit

• Refactor
  • Optimized internal props handling in table components for improved stability and performance.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The mergeProps function is optimized to use shallow cloning via Object.assign() instead of direct copying, and to return a single props object directly without unnecessary merging operations, while preserving overall merge semantics for subsequent props.

Changes

Cohort / File(s)	Summary
Props Merging Optimization packages/react-table/src/components/Table/base/merge-props.ts	Modified shallow cloning approach using Object.assign({}, props[0]) to avoid mutations. When single props object provided, returns directly without merge operation. Base of merge operation changed from empty object to firstProps, streamlining accumulation step while maintaining children cloning and className handling behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: refactoring the merge-props function to use Object.assign instead of mergeWith for cases without a custom merge function.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[patternfly-build]

Preview: https://pf-react-pr-12218.surge.sh

A11y report: https://pf-react-pr-12218-a11y.surge.sh

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@packages/react-table/src/components/Table/base/merge-props.ts`:
- Around line 15-23: The shallow clone via Object.assign({}, props[0]) still
shares nested references so mergeWith mutates props[0]; keep the early-return
for the single-prop case (return a shallow clone of props[0]) but when merging
multiple props call mergeWith with an empty object as the target (e.g.
mergeWith({}, props[0], ...restProps, customizer)) so the merge operates on a
fresh object and never mutates nested objects in props[0]; adjust references to
firstProps/mergeWith in merge-props.ts accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

lodash mergeWith mutate target object nested deep merge behavior

💡 Result:

• Yes—_.mergeWith mutates the first argument (object). It returns that same object after merging. [1][2]
• It performs a deep (recursive) merge of nested plain objects, similar to _.merge; sources are applied left→right and later sources win on conflicts. [2][3]
• Arrays are merged recursively by index by default (not concatenated). Example behavior: merging [{b:2},{d:4}] with [{c:3},{e:5}] produces [{b:2,c:3},{d:4,e:5}]. [3]
• customizer controls how values merge: it’s called with (objValue, srcValue, key, object, source, stack); if it returns undefined, Lodash falls back to default merge behavior. [1][2]
  • Common pattern: concatenate arrays by returning objValue.concat(srcValue) when objValue is an array. [1]

To avoid mutating an existing target, merge into a fresh object, e.g. _.mergeWith({}, target, source, customizer) (or clone first). [1][2]

Sources:
[1] Lodash mergeWith docs (note on mutation + customizer + array concat example)
[2] Lodash official docs excerpt (mergeWith signature + “mutates object”)
[3] Lodash merge docs (deep merge rules + array/object recursive merge example)

---

🏁 Script executed:

cat -n packages/react-table/src/components/Table/base/merge-props.ts

Repository: patternfly/patternfly-react

Length of output: 1490

---

Replace shallow clone with empty merge target to prevent nested mutation.

Object.assign is shallow, so nested objects in firstProps remain references to props[0]. When mergeWith then mutates firstProps recursively, it mutates the original nested objects in props[0], contradicting the "Avoid mutating the first prop collection" intent. Merge into an empty object instead to fully isolate the result.

🔧 Proposed fix

-  const firstProps = Object.assign({}, props[0]);
+  const firstProps = props[0];
   const restProps = props.slice(1);

   if (!restProps.length) {
-    return firstProps;
+    return Object.assign({}, firstProps);
   }

   // Avoid mutating the first prop collection
-  return mergeWith(firstProps, ...restProps, (a: any, b: any, key: any) => {
+  return mergeWith({}, firstProps, ...restProps, (a: any, b: any, key: any) => {

🤖 Prompt for AI Agents

In `@packages/react-table/src/components/Table/base/merge-props.ts` around lines
15 - 23, The shallow clone via Object.assign({}, props[0]) still shares nested
references so mergeWith mutates props[0]; keep the early-return for the
single-prop case (return a shallow clone of props[0]) but when merging multiple
props call mergeWith with an empty object as the target (e.g. mergeWith({},
props[0], ...restProps, customizer)) so the merge operates on a fresh object and
never mutates nested objects in props[0]; adjust references to
firstProps/mergeWith in merge-props.ts accordingly.

[jelly]

The AI bot is right, Object.assign only does a shallow clone so this wouldn't be the same.

[jelly]

An alternative might be windows.structuredClone() which is baseline.