Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
HichemTab-tech opened this issue Oct 26, 2024 · 1 comment
Open
Labels
react Related to the react adapter

Comments

@HichemTab-tech
Copy link

Version:

  • @inertiajs/react version: 2.0.0-beta.1

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:

  • In the source code:
    if (Array.isArray(incomingProp)) {
        pageResponse.props[prop] = [...((currentPage.get().props[prop] || []) as any[]), ...incomingProp]
    } else if (typeof incomingProp === 'object') {
        pageResponse.props[prop] = {
            ...((currentPage.get().props[prop] || []) as Record<string, any>),
            ...incomingProp,
        }
    }

You can check it here : source

  • If the incomingProp is an array, it is merged properly by appending the new elements to the existing ones.
  • If the incomingProp is an object, the properties are shallowly merged. This means that if the object contains an array (e.g., a pagination object with data, meta, and links), the array (data) is not merged correctly. Instead, it is replaced.

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:

  • When using Inertia::merge(), arrays inside objects (e.g., data within a pagination object) should also be merged, similar to how standalone arrays are merged.
  • Specifically, data within a pagination object should be appended to rather than replaced, while other properties (meta, links) should be replaced or handled appropriately.

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:

  • Implement a deep merging strategy for nested arrays within objects. For example, if an object contains an array property, that array should be merged rather than replaced.
  • Alternatively, allow developers to define custom merge behaviors for nested properties to prevent unintended replacements.
@YounesAmalou
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
react Related to the react adapter
Projects
None yet
Development

No branches or pull requests

2 participants