inertiajs / inertia

Inertia.js lets you quickly build modern single-page React, Vue and Svelte apps using classic server-side routing and controllers.
https://inertiajs.com
MIT License
6.55k stars 434 forks source link

Inertia::merge() Improperly Merges Pagination Objects in Inertia v2 #2068

Open HichemTab-tech opened 4 weeks ago

HichemTab-tech commented 4 weeks ago

Version:

Describe the problem:

In Inertia v2, the new merge feature allows developers to merge new data into existing props instead of replacing them. However, there is an issue with how arrays that are nested within objects are being handled. Specifically, when an incoming prop is an object containing an array (e.g., pagination data), the array is not being merged properly.

Current Behavior:

For example, consider a pagination object with the following structure:

{
    data: [/* items */],
    meta: { /* pagination metadata */ },
    links: { /* pagination links */ }
}

If you attempt to merge this object using Inertia::merge(), the data array will not be merged (appended to). Instead, it will be replaced by the new array, which leads to the loss of existing pagination items.

Expected Behavior:

Steps to Reproduce:

  1. Use Inertia::merge() in the backend to pass a pagination object as a prop.
  2. Observe how the data, meta, and links properties are merged in the frontend.
  3. Notice that while standalone arrays are merged correctly, the data array within the pagination object is replaced rather than appended.

Possible Solution:

YounesAmalou commented 2 weeks ago

Interesting observation, merge should actually merge the actual data object rather replacing it,

For example this might cause issues with paging if we try to send an object that contains the item list as an array + some meta data such as the max page, current page, next page...

Their might be other scenarios where the merge is actually needed.

This might not be convenient for people who expect to merge the actual data while not being familiar with this behavior,

I would suggest making this behaviors clear in the documentation while adding support for another method that handles the actual merge to keep both functions in case we actually need to replace the response.