I'm not sure why we were using deepMerge in the first place but I might be missing context. @oandregal, @straku any insights?

That's there for nested data. Here's a use case that breaks with these changes:

Before (current storybook):

After (with these changes): note how all other fields are cleared

Does this mean though that the deepMerge usage is correct? Shouldn't a nested field handle that on setValue or something (have a way to control how the changes merge in parent props)? Current implementation is buggy (the issue for tokens) and feels like opinionated handling of changes.

Take the following example, add a new value to a field type array:

What do you expect to receive as values in the onChange callback of DataForm?

For any other control (single items, not arrays) it streams the changed value, so I expected Book reviews, but the array control streams all (existing and new). That's the core issue we need to figure out. I see two ways to go about this:

1. make the array field type only stream the changes ("[ 'book reviews' ] ")
2. make the array field type stream the whole list of values ("[ 'photography' , 'book reviews']")

Whatever we do here, we should do for objects as well (when we support them).

If we go with 1, for nested scenarios, deepMerge would work as it is (for both arrays and objects). If we go with 2 (current behavior), we need to tweak the merging algorithm to do the following: for arrays (and objects), do not merge, just take the incoming data.

In general your option 1 can't work seamlessly because we can remove existing values, so we'd need to indicate that somehow without passing the whole new value.

Not sure how we can solve this (I'll have to familiarize myself more with the code), but it seems we can't have opinionated merging. I've mentioned a few times if setValue can help here, but in general for me it feels it should be the field that handles the merging somehow.

Hi @oandregal, If I am correct, are you saying need to update something like below?

diff --git a/packages/dataviews/src/stories/dataform.story.tsx b/packages/dataviews/src/stories/dataform.story.tsx
index 58b7226abc..ebafae27ed 100644
--- a/packages/dataviews/src/stories/dataform.story.tsx
+++ b/packages/dataviews/src/stories/dataform.story.tsx
@@ -2144,6 +2144,20 @@ const DataAdapterComponent = () => {
 			label: 'User Email',
 			type: 'email',
 		},
+		{
+			id: 'user.profile.tags',
+			label: 'User Tags',
+			type: 'array',
+			placeholder: 'Enter comma-separated tags',
+			description: 'Add tags separated by commas (e.g., "tag1, tag2, tag3")',
+			elements: [
+				{ value: 'astronomy', label: 'Astronomy' },
+				{ value: 'book-review', label: 'Book review' },
+				{ value: 'event', label: 'Event' },
+				{ value: 'photography', label: 'Photography' },
+				{ value: 'travel', label: 'Travel' },
+			],
+		},
 		// Example of adapting a data value to a control value
 		// by providing getValue/setValue methods.
 		{
@@ -2200,7 +2214,11 @@ const DataAdapterComponent = () => {
 		// Edits will respect the shape of the data
 		// because fields provide the proper information
 		// (via field.id or via field.setValue).
-		setData( ( prev ) => deepMerge( prev, edits ) );
+		setData( ( prev ) =>
+			deepMerge( prev, edits, {
+				arrayMerge: ( target, source ) => source,
+			} )
+		);
 	}, [] );
 
 	return (
@@ -2241,6 +2259,7 @@ const DataAdapterComponent = () => {
 							children: [
 								'user.profile.name',
 								'user.profile.email',
+								'user.profile.tags',
 							],
 						},
 					],

If not, Can you provide a example with type array which is nested type, that we can use here?

First, sorry for missing this scenario in the original PR 🙇

@hbhalodia @oandregal I like the idea of overwriting the original array with the new update. I don't see any obvious red flags for this approach.

Okay, so let's go with 2 (the current approach in the PR). While simple cases may work well for the 1 option (adding a single values), covering all the use cases will require a lot of complexity (think of: how do we rely that some values were removed?).

@hbhalodia yeah, thanks. An important note is to make sure that additional test would fail before this PR and that the changes her would fix it.

Yep, will update the PR soon with the above changes.

An important note is to make sure that additional test would fail before this PR and that the changes her would fix it.

Not able to collect this? Can you please explain what needs to be done related to test.

Note: Updated the changes.

Thanks,

Hi @oandregal @ntsekouras @straku, The PR is also updated with latest changes. Can you please take a look on it?

Thanks,