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

Fix objects merging in loadMore after refetch in usePaginationFragment #415

Conversation

DarynaAkhmedova
Copy link
Contributor

@DarynaAkhmedova DarynaAkhmedova commented May 7, 2024

The scope of this PR is to fix loadMore function from usePaginationFragment hook after we refetch data with different variables
that contain objects.

Problem
When we have an object as part of the default variables, for example filterBy: {property1: true} and the same object with extended params are passed to the refetch function like filterBy: {property1: true, property2: false}, we end up in the situation where previousVariables in useLoadMore hook is incorrectly combined.

In this case:

const previousVariables = {
    ...metadata.connection?.filterVariableDefaults,
    ...latestVariablesUsedByStandaloneRefetch,
    ...fragmentReference.__fragments,
}

metadata.connection?.filterVariableDefaults.filterBy = {property1: true}
latestVariablesUsedByStandaloneRefetch = {property1: true, property2: false}
fragmentReference.__fragments = {property1: true}

The end result will be missing second parameter from latestVariablesUsedByStandaloneRefetch
const previousVariables.filterBy = {property1: true}

To repro and test this scenario we added a new test case uses correct variable value in load next request when previous variable already contains an object that is different on refetch

Solution
To fix the problem we used merge function of lodash library to recursively merge objects at all levels. In this case we will ensures that every layer of nested properties is combined.

@DarynaAkhmedova
Copy link
Contributor Author

@alloy, @freiksenet, can you please have a look at this PR? :)

@freiksenet freiksenet merged commit f829f12 into microsoft:main May 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants