Skip to content
This repository has been archived by the owner on Jan 1, 2023. It is now read-only.

Store only active page state in the model #2

Open
MangelMaxime opened this issue Jul 25, 2018 · 5 comments
Open

Store only active page state in the model #2

MangelMaxime opened this issue Jul 25, 2018 · 5 comments

Comments

@MangelMaxime
Copy link

With the current AppState, you are storing a lot of information.

In general, when you are on the admin section , you don't need the Posts section state. For this reason, instead of creating one property in a record per children I prefer to use a DU.

Another reason in favor of this, is it's make the update function quicker. Because you can reduce the number of nested record and also need to copy less data.

// Proposition

type ActivePage =
    | Admin of Admin.Types.State
    | Posts of Posts.Types.State

type AppState = {
    // App's own state
    BlogInfo: Remote<BlogInfo>
    ActivePage: ActivePage
}
@Zaid-Ajaj
Copy link
Owner

That is what I thought at first too but storing the sub states is necessary: the top-level components sometimes need to "peek" into the data of other components in the UI tree to decide whether or not to add or remove functionality.

For example, the UI of Posts adds buttons "Edit" and "Delete" based on whether the user is logged in which is information present in Admin component (security token is present): component App (parent of Posts) peeks into Admin and propagates the information both to Posts and the sidebar.

Having a DU makes sense if the components are not related to each other, but this is not the case here.

Also, I am seperating information on current page from the state of the page: I don't want it to be possible to change the current page of the next state in my update functions to prevent inconsistencies, updating the current page happens only in reaction to url changes:

  • By invoking Url.navigate [ . . . ]
  • By manually changing the url in address bar

Components don't maintain their "current active page", that is decided from url and is propagated directly to the view functions:

// App/Admin/Backoffice -> View.fs
 
// currentPage comes from "outside" 
let render currentPage (state: State) dispatch = 
    match currentPage with
    | Home -> 
        homePage dispatch
    | NewPost ->
        NewArticle.View.render state.NewArticleState (NewArticleMsg >> dispatch)
    | Drafts -> 
        Drafts.View.render state.DraftsState (DraftsMsg >> dispatch) 
    | PublishedPosts -> 
        PublishedPosts.View.render state.PublishedPostsState (PublishedPostsMsg >> dispatch)
    | EditArticle articleId ->
        EditArticle.View.render state.EditArticleState (EditArticleMsg >> dispatch)
    | Settings ->
        Settings.View.render state.SettingsState (SettingsMsg >> dispatch)

Another reason in favor of this, is it's make the update function quicker. Because you can reduce the number of nested record and also need to copy less data.

Performance of modifying the state record is negligible: it is a simple operation that doesn't occur many times, unless I am running it 1000s of times, it not is important

@MangelMaxime
Copy link
Author

Components should avoid at most being related to each other. In fact, if they need to interact with each others, it should be throught Msg or externalMsg.

/// Child update
let update model msg : Model * Cmd<Msg> * ExternalMsg =
 // do something

/// Parent point of view
let update model msg =
    match msg with
    | Loaded (TabGeneral generalModel), GeneralInfoMsg subMsg ->  

        let (generalInfoModel, generalInfoCmd, externalMsg) = GeneralInfo.update generalModel subMsg

        let newModel =
            match externalMsg with
            | GeneralInfo.ExternalMsg.NoOp ->
                Loaded(originalData, TabGeneral generalInfoModel, toggleStateModel)
            | GeneralInfo.ExternalMsg.DomainUpdated domainInfos ->
                Loaded(domainInfos, TabGeneral (GeneralInfo.init domainInfos), toggleStateModel)

        newModel, Cmd.map GeneralInfoMsg generalInfoCmd

whether the user is logged in which is information present in Admin component (security token is present)

Store the User at the Root level and provide it directly to the update or view function that need it.

This point isn't clear for me.

Also, I am seperating information on current page from the state of the page: I don't want it to be possible to change the current page of the next state in my update functions to prevent inconsistencies, updating the current page happens only in reaction to url changes:

I think I understand what you mean, but my application is changing is Page state or current page only by changing the url either manually or using some helpers like Url.navigate

Performance of modifying the state record is negligible: it is a simple operation that doesn't occur many times, unless I am running it 1000s of times, it not is important

Depending on what you store in your model the performance are impacted. We saw it at work

Also, by storing each components state all the time you can easily show the previous state if you are not careful. Nothing force you to reset their state when the url changes.

@Zaid-Ajaj
Copy link
Owner

Components should avoid at most being related to each other

They are not related by the code and they don't know each other even exist, they are conceptually related

In fact, if they need to interact with each others, it should be throught Msg or externalMsg.

That is exactly what I am doing, see the "Message Inteception by example" in README, although I don't use external msg, I use just Msg it is the same thing, for example:

// Child -> Dispatches message "Logout"
| Logout -> state, Cmd.none 

// Parent -> Intercept child message "Logout" and update state without passing it down to child (only if needed) 
    | BackofficeMsg childMsg->
        match childMsg with 
        | Backoffice.Types.Msg.Logout -> 
            // intercept logout message of the backoffice child
            let nextState, _ = init()
            nextState, Urls.navigate [ Urls.posts ]
        | _ -> 
            match state.SecurityToken with 
            | Some token -> 
                let prevBackofficeState = state.Backoffice
                let nextBackofficeState, nextBackofficeCmd = 
                    // pass auth token down to backoffice
                    Backoffice.State.update token msg prevBackofficeState
                let nextAdminState = { state with Backoffice = nextBackofficeState }
                nextAdminState, Cmd.map BackofficeMsg nextBackofficeCmd
            | None ->
                state, Cmd.none

Store the User at the Root level and provide it directly to the update or view function that need it.

That is exactly what I am doing, I am storing SecurityToken : string option (the only user information needed) in Admin state and passing it in directly to the update Backoffice I don't need it at the very root (App), keeping it Admin is enough (See the section "Data Locality by example" in README)

I think we are actually doing the same things, I am just calling it different names, I need to see more large examples and compare. I learned these concepts by exploring different idea's and seeing how it works out, so I am curious how others are doing it as well 😄

@MangelMaxime
Copy link
Author

Indeed it's similar, for the access to the token. You still "break" the components independency because even if the Post doesn't pick directly into Admin.State the parent is.

From my point of view, this is the role of the parent to store the Session/Token info and dispatch it to the children that need it. In this case Post and Admin.

In theory, by doing that your components are more reusable (not much I do agree :) )

Sorry, I didn't read the whole code and only looked at the Type.fs so I missed the Message Inteception. In earlier version of my project I was doing the same however this have at least one problem. If you add a new message to intercept then the compiler can't help you.

Indeed you are using a | _ -> "default case" that will eat everything.

By having:

type ExternalMsg =
    | NoOp
    | SetUser of User

If I add a new case to ExternalMsg then the compiler will help we identify where I need to handle it.

Yes, we are indeed having the same idea just didn't implement the exact same structure :). This is a good news, as for me this ideas is one fo the missing pieces from the actual tutorials.

so I am curious how others are doing it as well 😄

This is planned since a long time for me to write a complexe application and open source the code to make a tutorial of it. I still need to create/update one or two libraries before being ready for it...

@Zaid-Ajaj
Copy link
Owner

Thanks for the clarification, I think I will adopt the ExtenalMsg approach, I like the explicitness of what will be handled within the child and what will be handled or propagated outside in the parent. Although I might call InterceptedMsg haha (I will think about a good name) 😄

From my point of view, this is the role of the parent to store the Session/Token info and dispatch it to the children that need it. In this case Post and Admin.

I know I could remove Admin altogether and leave Login and Backoffice as children of App and put the SecurityToken in the state of the App but again I don't feel it is necessary nor is it bad to have it there either

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants