Unexpected behaviour for logged-in user

and-pete commented 3 years ago

Hi Thomas,

I've identified what I think are two bugs, that have a chance of being related and so I'll keep them in the same issue.

See this Recordit GIF here for both behaviours demonstrated. For reference, this was recorded with me running the current commit in main right now.

Issue No. 1: Refreshing a logged-in user

When a logged-in user refreshes the page (for example, at Home), they present as though they are not logged in.

If while appearing logged-out they then:

Navigate away from the Home page to one of the navbar links available to logged-out users (Log in / Sign up) they can do so. But when they navigate back to Home from there they will now present as logged in.
Navigate away from the Home page directly to a route that has a version for logged-in users (e.g. Read more...), they will display as being logged in at that new route.

Experimenting, I've found a change that seems to partially fix this behaviour (i.e. it will display a refreshed user as being logged-in). In Main.purs, you currently write the the user profile to the currentUser Ref if there was a successful response from the server. If you add a second line where you also write it to the userBus Bus, upon page refresh you'll now initially see the the page display as logged out, but only for however many milliseconds it takes for the User request to come back and be decoded. And then you are displayed as logged in.

Adding the Bus.write changes another behaviour too. Toward the end of the GIF, I click the Settings link while displaying as logged in again (after a refresh & navigation changes), but it asks me to Sign In again. Writing to the Bus in main makes it so that the Settings page loads without being shown the Sign In page first.

Issue No. 2: `Initialize`-ation of the `Home` route

This is also related to users that are logged in. Specifically, I can't seem to be able to find a way to trigger this Just branch here in Home.purs in the component's Initialize action.

handleAction = case _ of
  Initialize -> do
    void $ H.fork $ handleAction LoadTags
    state <- H.get
    case state.currentUser of
      Nothing ->
        void $ H.fork $ handleAction $ LoadArticles noArticleParams

     -- This branch never triggers
      profile -> do
        void $ H.fork $ handleAction $ LoadFeed { limit: Just 20, offset: Nothing }
        H.modify_ _ { tab = Feed }

It doesn't matter whether I'm logged in + refreshing or whether I'm logged in + navigating back to the Home route from another route like an article.

You can see this at the end of the same GIF I linked above. I try to show that the page always shows the "Global Feed" (...that corresponds to the Nothing branch in the Initialize action) and never loads the "Your Feed" tab (...that should load if you had a Just profile in the component's state). I've checked by adding logging statements into the different branches and yeah there's always a Nothing value in state.currentUser when that initializing action runs.

After getting some help clearing up some confusion in Slack about the evaluation order of Halogen components a week or two back, I think this implies that the initialState input being received by the Home page's component on initialization must also be { currentUser: Nothing } (...as there's nothing else other than initialState that can modify the component's state before this Initialize action we're in). BUT... I'm trying to stick to observed behaviours here and not my interpretation of what's going on 😅

I'll take a look at some of the other components to see if I can find any equivalent behaviour. e.g. in the Editor component, it is similar to Home in that it is also built using the Connect.component HOC and its Initialize action starts with H.get-ing the current state.

My impression is that while components like Profile & ViewArticles (that aren't constructed using the HOC) need to read from the userEnv.currentUser/etc. Ref in order to get the state of the logged-in user, other components like Home above & Editor should be able to H.get a Just initial value form their component state when initializing as they'll have been provided some initial state as their input from the Connect.component HOC before the Initialize action runs. So if there's an issue where the HOC is not providing that initial state to Home, it might be observable in Editor too. I shall dig!

Sorry for the length!

The reason I've kept this as one GH issue is that I'm not sure that just adding Bus.write for the userBus in Main is the right fix for that refresh issue. And that there might be some single issue going on with HOC that's causing both.

thomashoneyman commented 3 years ago

Thank you for the detailed write-up! As soon as I have a little more time I’d like to explore what you’ve found, although my schedule is quite crunched for the next week or two (I’m moving!). Please don’t take silence on my part as ignoring the issue — I’d love to come back to this and I appreciate your time putting this together.

and-pete commented 3 years ago

No problem at all!

If I find anything else in the meantime while it's got me nerdsniped, I'll leave it here. Like... one other thing I've noticed by adding logging statements is that the Receive action in the HOC doesn't seem to ever be triggered either (not by login / logout / page refresh / nor route navigation).

Good luck with the move! :)

thomashoneyman commented 3 years ago

@and-pete Given how many of these issues seem tied to the bus implementation, perhaps we should go ahead with your Halogen 6 migration and switch to my state management library and see how many issues that resolves.

thomashoneyman commented 3 years ago

Fixed in #85! Thank you for the detailed write up!

thomashoneyman / purescript-halogen-realworld