elm/compiler

Changes in subscriptions without effect if model changes in response to command from init

Closed this issue ยท 20 comments

JaSpa commented

This is a problem with elm 0.19.0.

SSCCE

import Browser
import Html exposing (Html)
import Task
import Time exposing (Posix, Zone)

type Model
    = Loading
    | HasZone Zone

type Msg
    = NewZone Zone
    | Tick Posix

main =
    Browser.element
        { init = init
        , update = update
        , subscriptions = subscriptions
        , view = view
        }

init : () -> ( Model, Cmd Msg )
init _ =
    ( Loading, Task.perform NewZone Time.here )

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
    case Debug.log "update in response to" msg of
        NewZone zone ->
            ( HasZone zone, Cmd.none )
        Tick _ ->
            ( model, Cmd.none )

subscriptions : Model -> Sub Msg
subscriptions model =
    case Debug.log "subscriptions for" model of
        Loading ->
            Sub.none
        HasZone _ ->
            Time.every 1000 Tick

view : Model -> Html Msg
view model =
    Debug.log "view for" model
        |> always (Html.text "Nothing to see here, please look at the console.")

Ellie by @adeschamps showing the same problem: https://ellie-app.com/38yPbFdvv6qa1

Expected behaviour

In the browser's console the messages from Debug.log should appear in this order:

  1. view for Loading
  2. subscriptions for Loading
  3. update in response to NewZone
  4. subscriptions for HasZone
  5. view for HasZone
  6. update for Tick
  7. subscriptions for Tick
  8. view for Tick

with the last three steps repeated every second.

Actual behaviour

In the browser's console the following messages appear:

  1. view for Loading
  2. subscriptions for Loading
  3. update in response to NewZone
  4. subscriptions for HasZone
  5. view for HasZone

Analysis

  • Core problem: older subscriptions overwrite newer subscriptions.
  • Happens if: the command returned from the init function alters the model in such a way that the subscriptions change.
  • Guess on the problem location: _Platform_dispatchEffects. In the for-in loop is no ordering between subscriptions and commands.

More detailed breakdown

Expected

  1. Call to init getting the first model
  2. Setting subscriptions based on the first model
  3. Executing the command returned from init
  4. Call to update getting a new model
  5. Setting subscriptions based on the new model
  6. Executing the returned command
  7. etc. โ€ฆ

Actual

  1. Call to init getting the first model
  2. Executing the command returned from init
  3. Call to update getting a new model
  4. Setting subscriptions based on the new model
  5. Setting subscriptions based on the first model

This bug was first reported in #1762 by @jatinderjit.

I have encountered that same issue. This is really problematic ; is there anything that can be done to work around it?

Well, so far what I found is: never make conditional subscriptions. Not very satisfying and probably not optimal performance-wise, but eh.

I encountered the same problem and reported it here: elm/browser#49. I closed that issue since it is pretty much a duplicate of this one.

I think I'm running into the same thing. I have an app modeled pretty similarly to the elm-spa-example. The elm-spa-example uses subscriptions with a model argument, however they all seem to be using the data off the model so maybe it's not an issue there. What I'm seeing is my subscribe function being called with the correct model and then the port the subscription is based off of getting called with an entirely wrong model from before. The model being passed in exists nowhere in the state tree at all when viewing with the elm-debugger.

I also seem to be effected by this, my port was working fine in Elm 0.18, but now does send any messages unless it was subscribed during initial page load.

Seen a few people run into this now. The biggest issue with not being able to conditionally subscribe is that in development mode, the elm debugger becomes impossible to use if you are say, subscribing to animation frames or time.

I am having this same issue, but sometimes (~5-10% of the time) it randomly works. When I created a minimal example where the subscriptions are all initialized on page load (and no conditionals) it works as expected. This bug is a serious problem for any non-trivial application, like the one I am building here.

For me its ok if I change my subscription flag { model | animate = True } in a Msg generated directly by the UI. But if I do the same in an event from a Cmd, no dice.

Interesting this is working in my application, but now I'm terrified of making any changes that may trigger the bug! I was reading this guide: https://paulfioravanti.com/blog/2019/04/01/elm-018-to-019-upgrade-notes/ where Paul Fioravanti talks about it (and a workaround) and thought "Hey! I'm doing that and it works"

I have a few modes in my application (a walkie-talkie based on WebRTC, see here for demo:
https://cb.virtualairwaves.com/channel/1 ) and there are conditional subscriptions for them:

subscriptions : Model -> Sub Msg
subscriptions model =
    Sub.batch 
        [ get_user_media_except decodeGetUserMediaException
        , helo_success decodeHelloResponse
        , Browser.Events.onResize Resize
        , rtc_stream decodeGotRTCStream
        , rtc_success decodeRtcResponse
        , init_success decodeInitResponse
        , case model.push_to_talk of
            NotTalking ->
                Sub.none
            TimedOut ->
                Sub.none
            TalkTimer 0 ->
                Sub.none
            TalkTimer _ ->
                Time.every 250 TalkCountdown
        , if model.experiments.talk_timer_test then
            Time.every 5000 ToggleExTimerState
          else 
            Sub.none
        ]

The takeaway, I guess, is that this bug isn't going to be that easy to find. Not sure why it's working for me. I'll add a comment to myself that this may stop working at any time because of this bug.

Hi @VirtualAirwaves,

Actually the workaround (in case the bug appears to you) is quite simple (but not optimal). What I have done here is dispatch the subscription event always and treat it accordingly on update function. It is not optimal in terms of debug and maybe performance, but does the job.

Thanks, Raphael @raphaelpereira I just need to find another clever way to turn the timer on and off as needed (I'm using it to generated a progress/timer bar) to eliminate the overhead of having the timer tick every 250 milliseconds all the time.

            TalkTimer 0 ->
                Sub.none
            TalkTimer _ ->
                Time.every 250 TalkCountdown

I understand.
The ugly hack would be to use a port in case the bug ever triggers on your face. That way you would have an always subscription and the port would activated/deactivate the event (setInterval with a port send).
As I said it is an ugly hack, but as long as we have this bug I cannot thing on other solutions besides these two.

I can confirm this is also affecting routing. The subscriptions will become active one Msg in delay.

Is there anyway we can get more visibility to this issue? It seems like a critical bug that has been lingering for over a year. Its difficult to write well-structured elm code by subscribing to everything unconditionally.

Does it still reproduce? 0.19.1 just dropped.

Yes 0.19.1 is still affected by this. I just ran the SSCCE and it still produces the same results. There also is a pull request referenced in this issue, that hasn't been merged yet.

When I asked about this issue in Elm slack, Evan said it was a problem in elm/core.

I have experienced this bug myself. I have a question about it (maybe someone could confirm or deny it). Its not really about the init function right? All our examples of it involve the immediate state change after initialization, but thats just because those make for good SSCCE right? You could experience this bug well into an apps run time?

Yes I believe so. There is a comment in this thread about experiencing this issue when using Navigation.pushUrl. It also has an attached SSCCE.

I can confirm it is also happening in 0.19.1. None of my current 0.19.0 projects with conditional subscriptions and Navigation.pushUrl work anymore. Here is a SSCCE:

module Main exposing (main)

import Browser exposing (Document)
import Browser.Navigation as Navigation
import Html exposing (Html, a, div, li, text, ul)
import Html.Attributes exposing (href)
import Time
import Url
import Url.Parser as Parser exposing (Parser, s)


type alias Model =
    { navigation : Navigation
    , counter : Int
    }


type alias Navigation =
    { key : Navigation.Key
    , page : Page
    }


initialModel : Url.Url -> Navigation.Key -> Model
initialModel url key =
    { navigation =
        { key = key
        , page =
            case fromUrl url of
                Nothing ->
                    NotFound

                Just a ->
                    a
        }
    , counter = 0
    }


parser : Parser (Page -> b) b
parser =
    Parser.oneOf
        [ Parser.map Home Parser.top
        , Parser.map About (s "about")
        ]


fromUrl : Url.Url -> Maybe Page
fromUrl url =
    -- Treat fragment as path for convenience
    { url | path = Maybe.withDefault "" url.fragment, fragment = Nothing }
        |> Parser.parse parser


type Page
    = Home
    | About
    | NotFound


type alias Flags =
    ()


init : Flags -> Url.Url -> Navigation.Key -> ( Model, Cmd Msg )
init flags url key =
    ( initialModel url key, Cmd.none )



-- UPDATE


type Msg
    = UrlRequested Browser.UrlRequest
    | UrlChange Url.Url
    | Tick Time.Posix


update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
    case Debug.log "" msg of
        UrlRequested urlRequest ->
            case urlRequest of
                Browser.Internal url ->
                    ( model, Navigation.pushUrl model.navigation.key (Url.toString url) )

                Browser.External href ->
                    ( model, Navigation.load href )

        UrlChange url ->
            let
                navigation =
                    model.navigation
            in
            ( { model
                | navigation =
                    { navigation
                        | page =
                            case fromUrl url of
                                Nothing ->
                                    NotFound

                                Just a ->
                                    a
                    }
              }
            , Cmd.none
            )

        Tick _ ->
            ( { model | counter = model.counter + 1 }, Cmd.none )



-- VIEW


view : Model -> Document Msg
view model =
    { title = "hello title"
    , body = [ bodyView model ]
    }


bodyView : Model -> Html Msg
bodyView model =
    div []
        [ text "hello world"
        , ul []
            [ li [] [ a [ href "#/" ] [ text "Home" ] ]
            , li [] [ a [ href "#/about" ] [ text "About" ] ]
            ]
        , div []
            [ case model.navigation.page of
                NotFound ->
                    text "Page not Found"

                Home ->
                    Html.div
                        []
                        [ text "Home page"
                        , Html.div
                            []
                            [ text <| String.fromInt model.counter ]
                        ]

                About ->
                    text "About page"
            ]
        ]



-- SUBSCRIPTIONS


subscriptions : Model -> Sub Msg
subscriptions model =
    case model.navigation.page of
        Home ->
            Time.every 1000 Tick

        _ ->
            Sub.none



-- INIT


main : Program Flags Model Msg
main =
    Browser.application
        { init = init
        , update = update
        , view = view
        , subscriptions = subscriptions
        , onUrlRequest = UrlRequested
        , onUrlChange = UrlChange
        }

Note that you have to click twice in the Home link to make the counter work again after visiting a different route.

And here is the Slack thread with some findings:

https://elmlang.slack.com/archives/C13L7S5GR/p1572031426192200

TL;DR
Apparently it might be something related to the order of the definitions/assignments within the compiled javascript

With the changes in elm/core@704dcc0 I am seeing the expected behavior with your SSCCE.

The fix should come out with elm/core 1.0.3 along with some other improvements. In the meantime, if you are running into this issue specifically with a task like Time.here or Time.now the advice from Ossi here elm/core#993 (comment) should help you get around it. Using Process.sleep can get around it in some cases. E.g. Process.sleep 1 |> Task.andThen (\_ -> Task.now)