Pls review my (basic) SPA example

Hello Elmers,

Up until now, I hadn’t managed to get a basic SPA working when playing with Elm.

I think some things finally clicked for me this weekend though, so I’m looking at getting some feedback before expanding further on a little example/demo.

My project is available here: elm-spa-example (commit 2a33a82)

And running live : here.

Although I did start following a tutorial, it was designed for Elm prior to v0.19 so I got stuck very quickly and diverged. So I’d say that 90% of the final product is unrelated to this tutorial.

A few things I’d like to get feedback on:

  • is the code readable?
  • easy to follow?
  • could I simplify some areas?
  • use better naming strategies?
  • organize code better?
  • did I obviously misunderstand some concepts?
  • anything I didn’t think about?

If you have the time, I’d appreciate any feedback. Thanks :slight_smile:

2 Likes

I love how Elm applications are so similar by default :grin:

One small advice - I noticed you’re using type alias for unboxed primitives (type alias Email = String) this can be a bit tricky since it won’t prevent you from throwing something else at it…

On my projects I try to either use records for when you’re just trying to make the argument more easy to read (User { email : String }) or if I need to pass this value around in a safe way I usually create a custom type with just one variant (type Email = Email String) so I can’t pass a different string by mistake.

Thanks for the feedback @georgesboris

I was trying to improve readability by creating a type (alias) Email.

Defining type Email = Email String works even better! I found I had to pattern match to extract the underlying String (see below). Can it be done any other way?

type Email
    = Email String


type User
    = Guest
    | User Email


getEmail : User -> Maybe String
getEmail user =
    case user of
        Guest ->
            Nothing

        User email ->
            case email of
                Email str ->
                    Just str

you can use a nested pattern here

getEmail : User -> Maybe String
getEmail user =
    case user of
        Guest ->
            Nothing

        User (Email email) ->
            Just email
1 Like

Much nicer! Thanks @Carsten_Konig

1 Like

@benjamin-thomas Any place where you are using a custom type with a single variant, like type Email = Email String it means that pattern can be deconstructed without pattern matching. @Carsten_Konig showed how you can reduce the branching when it’s wrapped inside the User type, but an even simpler example could be the following function, where the parameter is deconstructed in the argument

address : Email -> String
address (Email emailAddress) = emailAddress
1 Like

yes exactly - IMHO this is a common pattern although I’d suggest calling it toString instead of address - this is especially nice if you opt to add the type into it’s own module - so you can keep the data-constructor private:

module Email exposing (Email, fromString, toString)

type Email
    = Email String

fromString : String -> Email
fromString = Email

toString : Email -> String
toString (Email email) = email

and if you want to constrict the valid values you can switch this into fromString : String -> Maybe Email for example (of course you’d need a bit of code to check the value - probably with a regex)

1 Like

Very nice! Thank you both @mattpiz and @Carsten_Konig, getting some great tips here ^^


One thing that’s been bugging me slightly is the verbosity of my update function in Main.elm. The transformation feels almost mechanical.

update msg model =
    ...
    case ( msg, model.page ) of
        ( NewPostMsg subMsg, NewPostPage subModel ) ->
            let
                ( newModel, newCmdMsg ) =
                    Page.Post.New.update subMsg subModel
            in
            ( { model | page = NewPostPage newModel }, Cmd.map NewPostMsg newCmdMsg )

        ( ListPostsMsg subMsg, ListPosts subModel ) ->
            let
                ( newModel, newCmdMsg ) =
                    Page.Post.List.update subMsg subModel
            in
            ( { model | page = ListPosts newModel }, Cmd.map ListPostsMsg newCmdMsg )

        ( ShowPostMsg subMsg, ShowPost subModel ) ->
            let
                ( newModel, newCmdMsg ) =
                    Page.Post.Show.update subMsg subModel
            in
            ( { model | page = ShowPost newModel }, Cmd.map ShowPostMsg newCmdMsg )

        ( _, _ ) ->
            ( { model | page = NotFound }, Cmd.none )

So I tried to introduce a closure to clean things up:

update msg model =
    let
        newPage nextPage ( subUpdate, subMsg, subModel ) currMsg =
            let
                ( newModel, newCmdMsg ) =
                    subUpdate subMsg subModel
            in
            ( { model | page = nextPage newModel }, Cmd.map currMsg newCmdMsg )
    in
    case ( msg, model.page ) of
        ( NewPostMsg subMsg, NewPostPage subModel ) ->
            newPage NewPostPage ( Page.Post.New.update, subMsg, subModel ) NewPostMsg

        ( ListPostsMsg subMsg, ListPostsPage subModel ) ->
            newPage ListPostsPage ( Page.Post.List.update, subMsg, subModel ) ListPostsMsg

        ( ShowPostMsg subMsg, ShowPostPage subModel ) ->
            newPage ShowPostPage ( Page.Post.Show.update, subMsg, subModel ) ShowPostMsg

I intend to read it as such:

Build a newPage named NewPostPage, given this related data: (update, msg, model) and map the result to my current Msg scope with NewPostMsg.

For this to work with every msg however, Page has to have the same “shape”, meaning this function
won’t play well with Home or LoginPage as-is.

type Page
    = Home
    | LoginPage
    | NewPostPage Page.Post.New.Model
    | ListPostsPage Page.Post.List.Model
    | ShowPostPage Page.Post.Show.Model
    | NotFoundPage

Also I added “Page” as a suffix to every variant of Page because I otherwise feel slightly confused with name clashing between Route, Msg and Page (when reading or writing code).

In the golang community, this practice is called “stuttering” and generally frowned upon. But I feel it may be necessary here and maybe even a good idea?

Anybody got tips here?

1 Like

Rather than having newPage take the update function along with all of the arguments to call that update function, you could apply the update function directly and give the result to newPage. I usually call that function updateWith.

Would look something like this:

update msg model =
    case ( msg, model.page ) of
        ( NewPostMsg subMsg, NewPostPage subModel ) ->
            Page.Post.New.update subMsg subModel
                |> updateWith NewPostPage NewPostMsg model

        ( ListPostsMsg subMsg, ListPostsPage subModel ) ->
            Page.Post.List.update subMsg subModel
                |> updateWith ListPostsPage ListPostsMsg model

        ( ShowPostMsg subMsg, ShowPostPage subModel ) ->
            Page.Post.Show.update subMsg subModel
                |> updateWith ShowPostPage ShowPostMsg model

        _ ->
           ( model, Cmd.none )

updateWith toModel toMsg model (pageModel, pageCmd) =
    ( { model | page = toModel pageModel }
    , Cmd.map toMsg pageCmd
    )

Or you could use a closure to eliminate the model argument:

update msg model =
    let
        updateWith toModel toMsg (pageModel, pageCmd) =
            ( { model | page = toModel pageModel }
            , Cmd.map toMsg pageCmd
            )
    in
    case ( msg, model.page ) of
        ( NewPostMsg subMsg, NewPostPage subModel ) ->
            Page.Post.New.update subMsg subModel
                |> updateWith NewPostPage NewPostMsg

        ( ListPostsMsg subMsg, ListPostsPage subModel ) ->
            Page.Post.List.update subMsg subModel
                |> updateWith ListPostsPage ListPostsMsg

        ( ShowPostMsg subMsg, ShowPostPage subModel ) ->
            Page.Post.Show.update subMsg subModel
                |> updateWith ShowPostPage ShowPostMsg

        _ ->
           ( model, Cmd.none )

I’ve found in practice that pages without models don’t have any msgs to handle either. So they’re just handled by the catch-all _ -> (model, Cmd.none) at the end of the case.

In theory a page could turn a Msg directly into a Cmd without manipulating a model, but I’ve never needed to do that personally. If I did, I’d probably just write the transformation manually for that one case branch. If it did happen frequently, I’d write another updateWith function that handled pages without models.

This is what I do. I have PageModel with constructors like PageModelLogin & PageModelAccount, and PageMsg with constructors like PageMsgLogin & PageMsgAccount. It’s a bit verbose, but seems to help keep everything clear.

1 Like

Very nice, thank you @Jayshua!

I like this method, it clears up a lot of clutter and the update function becomes much more readable that way.

Since updateWith is very specific to changing Main's model page attribute, how do you handle applying other transformations to Main's model from the page’s model?

I found I had to “expand” again the function execution to access newModel.user and couldn’t really find better way to write this.

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
    case ( msg, model.page ) of
        
        -- OK
        ( NewPostMsg subMsg, NewPostPage subModel ) ->
            Page.Post.New.update subMsg subModel
                |> updateWith NewPostPage NewPostMsg model

        -- Ouch
        ( LoginMsg subMsg, LoginPage subModel ) ->
            let
                ( newModel, newCmdMsg ) =
                    Page.Creds.Login.update subMsg subModel
            in
            updateWith LoginPage LoginMsg { model | user = newModel.user } ( newModel, newCmdMsg )

I think there are three main options here.

Option #1: Custom Cmd Types

Instead of returning a Cmd Page.Msg from your Page.update function, return a custom Action Page.Msg type. This type can be deconstructed in the updateWith function.


type Action msg
	= EmbeddedCmd (Cmd msg)
	| Login User

...

updateWith toModel toMsg model (pageModel, pageAction) =
	case pageAction of
		EmbeddedCmd cmd ->
			({model | page = toModel pageModel}, Cmd.map toMsg cmd)

		Login user ->
			({model | page = toModel pageModel, user = user}, Cmd.none)

You can define all the standard Cmd functions in Action.elm:

-- In Action.elm

batch : List (Action msg) -> Action msg

map : (a -> b) -> Action a -> Action b

-- This one means you don't have to pass Browser.Navigation.Key to sub-pages.
-- It's unwrapped in Main and the navigation key is retrieved
-- from the main model in updateWith.
navigate : String -> Action msg

-- This one triggers a msg after some number of milliseconds.
delay : Float -> msg -> Action msg
delay milliseconds msg =
    EmbedCmd (Task.perform (\() -> msg) (Process.sleep milliseconds))

-- This one means you don't need to directly access ports in sub-pages.
-- Instead the port is used in Main when the Action is deconstructed.
sendEvent : Value -> Action msg
sendEvent =
    SendEventToJs -- This is a constructor of the Action type

-- It's also possible to get request-response like behavior with ports
-- When `SendRequestToJs` is deconstructed, updateWith will put the Decoder in a Dict in the main
-- model and send the Value out a port along with the dict key. The JS will do
-- something and send the response with that key, which Main will match up with the
-- decoder stored the Dictionary to get the Msg to update with.
-- Note that Decoders have functions inside of them, so storing them in your Model
-- in this way means that your Model can't be serialized using the Elm debugger.
-- I've personally found the trade-off to be worth it.
sendRequest : Value -> Decoder msg -> Action msg
sendRequest =
    SendRequestToJs

A (common?) variant of Option #1 is to return a triple instead of a tuple.

Page.update : Msg -> Model -> (Model, Cmd Msg, Action)

This avoids having to wrap Cmds in Action.EmbeddedCmd and is easier to introduce into an existing application. Personally, I’ve found transitioning entirely over to an Action type and never using Cmd in pages to be the better option. Just re-define any Cmd producing functions you need so that they return an Action instead of a Cmd.

Option #2: Mapping Msgs

You could also write the Page.update function like this:


type Msgs model msg =
	{ user : User
	, toMsg : Msg -> msg
	, setUser : User -> model -> model
	, doSomething : String -> Int -> msg
	}

Page.update : Msgs model msg -> model -> Model -> Msg -> (model, Cmd msg)

In this way the Page.update function deals with the main model & msgs directly, but through acessor, setter, and constructor functions. You can give it access to root-level msgs that it might need to create by adding more functions to the Msgs record. Same with manipulating the root-level Model by adding more functions of the form model -> model.

This seems to produce a lot of boilerplate in my experience. Overall Options 1 & 3 seem to work better.

Option #3: Flatten your Model

Another solution is to not have sub-page models or sub-page msgs. This doesn’t always make sense, but I have one relatively large application that just passes the root Model down to every sub-page, and they generate root-level Msgs rather than page-specific ones. The page field on the Model is just a discriminator without any additional data:


type Page
	= ContactsPage
	| EventsPage
	| StudentsPage
	| SettingsPage

This works quite well for that application. I think the reason is the entire application is manipulating a single “project file”. Lots of apps are CRUD like with pages such as “Index Students”, “Show Student”, “Index Courses”, “Show Course”, “Create Course”, etc. Each of these pages is more-or-less a completely independent thing.

The application I use this architecture in isn’t really like that. Instead, there’s a single “Project File” that behaves more like a Word Document or an Excel File. Users open the project, update data, and save the project. Each “page” in the Elm app is just viewing and manipulating a different sub-set of the project file.

A side-effect of this that might not be immediately obvious is that pages no longer have their own update functions. Instead, the root update function handles everything. (It can still delegate to other functions, but you get to decide how to break it down based on the needs of the code rather than the needs of ux-dictated page breakdowns.)

Another benefit is not losing page state like partially filled forms when switching between pages. This is usually a win from a UX standpoint.

The application I just described is embedded in a bigger app for manipulating user accounts, permissions, project file management etc. That wrapping application is much more CRUD like and uses Option #1 to great success. Given a large enough application, the options aren’t mutually exclusive.

Sorry for the late feedback @Jayshua, I’ve been absorbed learning a bit of Haskell over the weekend believe it or not, I think I may have been bitten by a functional bug :stuck_out_tongue:

Option #1 looks appealing to me but I’m not grokking it yet. I may have to let it sink in though :slight_smile:

I found a reference to option #2 here: elm-patterns/global-actions.md at master · sporto/elm-patterns · GitHub

And option #3 sounds great indeed. I had a question regarding this kind of setup a few months back, but I didn’t really know how to express my needs then. This setup sounds great, I’ll keep it in mind!

Regarding option #2, what would help me would be to see an example of this design. Do you have something you could point me too?

Many thanks

in the past the Elm community has written a number of posts about app-page (or app-component) communication. Since they are great posts I think they are worth to link them here:

https://folkertdev.nl/blog/elm-child-parent-communication/
https://folkertdev.nl/blog/elm-child-parent-communication-continued/

Thanks very much @passiomatic.

I have to play more with Elm to be really comfortable with message passing, so those posts are perfect for me!

I adapted the candy and allowance to the latest elm syntax easily so that’s the one I’ll study first.

I faced some difficulties adapting the translator pattern on first try, so I’ll come back to it a bit later :slight_smile:

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.