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

PushUpdates causes all viewmodels of same client instance to push the changes #325

Open
DG4ever opened this issue Jan 16, 2024 · 3 comments

Comments

@DG4ever
Copy link

DG4ever commented Jan 16, 2024

I am using the useConnect hook to connect to different viewmodels from the same client.
It seems that when I do "PushUpdates" in one of the viewmodels, the changes of both viewmodels are transmitted to the client.
I dont't think this is the correct behaviour or am I doing something wrong here?

const { vm, state } = useConnect<State>(viewModelName, {},
    {
        vmArg: {
            ...initialViewModel
        },
        headers: { Origin: window.location.pathname, UrlPattern: urlPattern, UrlParams: urlParams, Authorization: "Bearer " + window.localStorage.getItem("access_token") },
        exceptionHandler: exceptionHandler
    }
);

For me this results in some problems:
In one viewmodel I am using a timer to poll some data and update the view via "PushUpdates".
In the other viewmodel I am calling a long operating function that fetches some data and assign it to multiple propertys of the viewmodel.
Now if the "PushUpdates" of the first viewmodel is called during that function call of the second viewmodel, half of the data of the second viewmodel is pushed to the client before all data is fetched. Of course the missing data is pushed after the function is finished but in my view I expect to get all data at once.

Is this the intended behaviour of "PushUpdates" or might this be a bug?

@dsuryd
Copy link
Owner

dsuryd commented Jan 16, 2024

Indeed, PushUpdates by design will send updates from all active view models. In your case, you may need to implement synchronization between the multiple PushUpdates, or in the long-running function, to keep the changes in a local object until you're ready to set everything to the view model properties in one go.

@DG4ever
Copy link
Author

DG4ever commented Jan 17, 2024

Thank you for the fast respone.
I did not expect this behaviour, it seems quite counterintuitive to me (at least if the viewmodels don't interact with each other).
I just looked into the VMController and all changed viewmodels also get pushed at the end of a client request:

// Push updates on other view model instances in case they too change.
foreach (var vmInfo in _activeVMs.Values.Where(vm => vm.Instance != vmInstance))
PushUpdates(vmInfo);
});

// If this request causes other view models to change, push those new values back to the client.
foreach (var vmInfo in _activeVMs.Values)
PushUpdates(vmInfo);
});

For the active PushUpdates the corresponding code seems to be this one:

private void VmInstance_RequestPushUpdates(object sender, EventArgs e)
{
foreach (var vmInfo in _activeVMs.Values)
PushUpdates(vmInfo);
}

Is there an easy way for me to override the VMController?
I would like to change this behaviour.

Maybe an option to disable this behaviour and force it via a parameter in "PushUpdates" would be useful?

@dsuryd
Copy link
Owner

dsuryd commented Jan 18, 2024

This was designed this way to ensure multiple view models stay in sync with each other. For example, in a list-detail pattern, a list selection change must also trigger the detail update. A possible solution for your use case is to add a new type of view model that the general PushUpdates would simply ignore and let it initiate its own updates in isolation, but that would require changes in a few areas of the framework, but truthfully I would rather this be solved in the application level per my previous suggestions.

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

No branches or pull requests

2 participants