elm / core

Elm's core libraries
http://package.elm-lang.org/packages/elm/core/latest
BSD 3-Clause "New" or "Revised" License
2.8k stars 359 forks source link

_Platform_gatherEffects can throw runtime exception - Maximum call stack size exceeded #1123

Open rupertlssmith opened 3 years ago

rupertlssmith commented 3 years ago

Elm 0.19.1

elm.js:2166 Uncaught RangeError: Maximum call stack size exceeded
    at _Platform_gatherEffects (elm.js:2166)
    at _Platform_gatherEffects (elm.js:2179)
    ...

This function is not tail recursive when working with a nested Cmd.batch. Large flat batches are ok, since a for loop is used to iterate those.

https://github.com/elm/core/blob/master/src/Elm/Kernel/Platform.js#L273

Workarounds:

  1. Don't create large nested Cmd.batch.
  2. OR Users can write their own custom version of Cmd.batch that keeps the batch flat, and gets transformed into a Cmd.batch later one, once the whole batch has been built.

===

It is conventient to use an update monad like this:

andThen :
    (model -> ( model, Cmd msg ))
    -> ( model, Cmd msg )
    -> ( model, Cmd msg )
andThen fn ( model, cmd ) =
    let
        ( nextModel, nextCmd ) =
            fn model
    in
    ( nextModel, Cmd.batch [ cmd, nextCmd ] )

When operating over a large number of update functions, which are written in this style:

someUpdate : ... -> Model -> ( Model, Cmd Msg )

It can be convenient to create a larger update by folding over a list of smaller ones, like this:

   List.foldl
        (\... accum ->
            andThen
                (someUpdate ...)
                accum
        )
        ( model, Cmd.none )

Even if the smaller someUpdate functions return Cmd.none, a large nested Cmd.batch will be created.

This can be fixed by avoiding using this style of programming, but it seems a shame to not allow it.

Solutions:

  1. Make _Platform_gatherEffects tail recursive.
  2. OR Make Cmd.batch a bit smarter, by having it flatten any nested batching.
github-actions[bot] commented 3 years ago

Thanks for reporting this! To set expectations:

Finally, please be patient with the core team. They are trying their best with limited resources.

avh4 commented 3 years ago

How deep of a nesting are you talking about? I thought current browsers typically allow 10,000-30,000 levels of stack calls. Do you have thousands of updates you're composing, or is this happening with just tens or hundreds of nesting levels?

turboMaCk commented 2 years ago

Also note that:

andThen :
    (model -> ( model, Cmd msg ))
    -> ( model, Cmd msg )
    -> ( model, Cmd msg )

is not a describing monad. As you can see it's (a -> X a) -> X a -> X a while point of nomadic bind (andThen) is to go from X a to X b. I would suggest not to go crazy about similar abstractions especially those which involve recursion in update.