mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.24k stars 141 forks source link

[Question] Best practise for CRUD operations #286

Closed dylanbarquilla closed 2 years ago

dylanbarquilla commented 2 years ago

Hello, we use Fluxor in our project and it works like a charm.

We saw the following tweet (why we should use Cascading Value in some cases), and we noticed in the thread the following statement:

Of course, if you use Fluxor then you don't need to worry about passing parameters or cascading values, and your components will only re-render when they need to :)

It brings a doubt in the approach we have in our project. Let me present you the global approach that we took, with a simplified example. We have a basic "house" store, with this simple state: public record HouseState(HouseFull House);

What we usually do is:

  1. Having a Blazor page ("HouseDetails"), with:
    a. public HouseFull CurrentHouse => HouseState.Value.House; (code part) b. <HouseCrud House="CurrentHouse"></HouseCrud> (razor part)

  2. A Blazor Component "HouseCrud" with: a. [Parameter] public HouseFull House{ get; set; } = default!; b. HouseInput? HouseInput { get; set; } = null; => this is the object that we use in the Razor part to show information and enable edition on fields c. And this it to create the input each time the Parameter of the component if modified:

    protected override void OnParametersSet()
    {
    if (House is null) return;
    if (HouseInput is null) this.HouseInput = new();
    
    HouseInput.UpdateFromModel(House);
    
    base.OnParametersSet();
    }

    So the "simplified" workflow when the user click on the "Save" button on the HouseCrud component is:

  3. Dispatch the HouseUpdateAction(HouseInput) with the input

  4. In the store (fast writing): effect method -> call API to save entity -> await the result, and dispatch LoadHouseByIdAction in order to update the state with the new House -> call API to load entity and dispatch the HouseSetHouseAction with the result -> The reducer save the new State

  5. HouseState is updated, so it forces the page to re render

  6. The CurrentHouse property of the Blazor page now return a new version of the state

  7. So the HouseCrud component has its parameter House changed, so the OnParametersSet method is called (see above)

  8. The HouseInput is up to date, so the user can see the new data in the form (and everybody is happy, right?).

But with the twitter thread, it let us imagine an other approach:

  1. Having a Blazor page ("HouseDetails"), with:
    a. <HouseCrud></HouseCrud> (no parameter)

  2. A Blazor Component "HouseCrud" with: a. public HouseFull House=> HouseState.Value.House; b. HouseInput? HouseInput { get; set; } = null; c. No needs of OnParametersSet() anymore, but OnAfterRender(bool firstRender) instead, which will be fired when the HouseState will be updated.

    protected override void OnAfterRender(bool firstRender)
    {
    if (House is null) return;
    if (HouseInput is null) this.HouseInput = new();
    
    HouseInput.UpdateFromModel(House);
    
    base.OnParametersSet();
    }

Which approach seems the best for you, and which one would you recommend? More globally is it a good practice to do not use Parameter in components, but only use the Fluxor state?

Thanks a lot in advance, //Dylan

mrpmorris commented 2 years ago

Email me some dates and times you are available and we can set up a video call, assuming it's okay for me to see the source?

bradtwurst commented 2 years ago

@dylanbarquilla if you and @mrpmorris connect and review, please provide results so that others may also benefit

dylanbarquilla commented 2 years ago

Hello @mrpmorris, I just sent you an email. Thanks again a lot for your availability!

@bradtwurst Yes, it was planned to share the results with the community (maybe I will be able to write an example/sample with results)

mrpmorris commented 2 years ago

The requirement was to make a call to a server and then allow the UI to edit the mutable DTO returned via the API.

Dylan was uncomfortable storing this DTO in the store in order for the UI to edit it because it is mutable.

The correct approach is as follows

  1. Effect receives DTO from server
  2. It dispatches an action that has a property set to the DTO
  3. The DTO is not put into the store
  4. UI injects [Inject] private IActionSubscriber ActionSubscriber { get; set; }
  5. UI has a property of that DTO type
  6. In OnInitialized you call ActionSubscriber.SubscribeToAction<ThatActionType>(this, action => ThatDtoProperty = x.DtoFromServer)
  7. UI implements IDisposable and calls ActionSubscriber.CancelAllSubscriptions(this);

There is an example here -> https://github.com/mrpmorris/Fluxor/tree/master/Tutorials/01-BasicConcepts/01E-ActionSubscriber

uhfath commented 2 years ago

Thank you for sharing this use case with us. But I have a question. What would be the best approach if one needs an async action in a subscriber to be performed? Such as passing a DtoFromServer to a component which supports only an async update method?

mrpmorris commented 2 years ago

You can just throw away the task, as the subscriber won't wait for it anyway

_ = DoAsync(action.Data);

If you need to await it, you can make your method async void. 99% of the time that's poor practice in C#, but this would be a 1% case as we know for a fact the caller will never need to await your method.

uhfath commented 2 years ago

I'm actually concerned more about exception handling in this case. Will they be caught in Fluxor.Blazor.Web.StoreInitializer.UnhandledException or somewhere else? It's said here that exceptions will be propagated to AppDomain.UnhandledException. Can't test this right now, so not sure as how it will behave.

mrpmorris commented 2 years ago

You should be using effects for this

uhfath commented 2 years ago

Wouldn't it mean linking effects, which should just do the logic with UI components, which just need to use results of effects?

mrpmorris commented 2 years ago

Why does the component perform an async update operation?

uhfath commented 2 years ago

Hard to tell since it's up to a component's author to decide. Can't say. I remember that one of commercial components that we used had this and previously I've seen same for one or two open source ones. Perhaps it could be related to InvokeAsync(StateHasChanged) being used. Not sure. What are the reasons for events to be strictly sync?

mrpmorris commented 2 years ago

Because async options should happen outside of the Fluxor dispatch flow so it can accept additional actions without having to wait too long for previous ones to finish.

uhfath commented 2 years ago

Got it. Thanks! I think a link to this issue or some snippets of it should be part of tutorials.