rtfeldman / elm-spa-example

A Single Page Application written in Elm
https://dev.to/rtfeldman/tour-of-an-open-source-elm-spa
MIT License
3.28k stars 530 forks source link

reduce `toSession` proliferation #79

Open Hermanverschooten opened 5 years ago

Hermanverschooten commented 5 years ago

In the code as supplied here, each page needs to have a toSession method exposed that essentially just returns the session field out of its model. Why not make this more generic?

getSession: { a | session: Session } -> Session
getSession =
    session

Then you can replace the calls in the case statement:

        ...
        Home home ->
            Home.toSession home

        Settings settings ->
            Settings.toSession settings

with

        ...
        Home home ->
           getSession home

        Settings settings ->
            getSession settings

And there is no longer a need to have a toSession in the pages, unless you want it.

Sircular commented 4 years ago

On that note, why have the session as part of the individual page models if it needs to be shared across all of them? Why is it not part of the top-level model?

Hermanverschooten commented 4 years ago

The model here is a Type, not a Type alias, and you need the information in every page anyway, so you would have to pass it along.