hyperapp: Infinite loop when subscription immediately dispatches

I have a Countdown subscription, which for simplicity, looks like this:

const CountdownFx = (dispatch, { id, seconds, progress, complete }) => {
  let remaining = seconds + 1; // this is contrived to make this example smaller
  let handle = null;
  const tick = () => {
    remaining -= 1; // again, contrived
    if (remaining > 0) {
      dispatch(progress, { id, remaining });
      handle = setTimeout(tick, 1000);
    } else {
      dispatch(complete, { id });
    }
  }

  tick();

  return () => clearTimeout(handle);
}

export default props => [CountdownFx, props]

When I enable this countdown, it triggers setState immediately, which runs subscriptions immediately, and does an infinite loop.

If I make a timeout around the initial tick, everything works fine.

My real example makes more sense that tick happens immediately, but this example also triggers the problem.

The problem is here.

A solution (which is maybe incorrect) is to mimick the render deferring for subscriptions, where running the next subscription patch is deferred/locked.

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 37 (37 by maintainers)

Most upvoted comments

@zaceno Here are some answers:

Things which require immediate dispatch should be implemented with effects.

Yes, or without them. To avoid reading from the global scope, e.g., window, inside your init function, I suggest creating an HOA as I described in https://github.com/jorgebucaran/hyperapp/issues/857#issuecomment-508838282. What’s nicer, an HOA to init the app with default parameters.

export const withDefaults = params => app => props =>
  app({
    ...props,
    ...{ init: () => props.init(params) }
  })

import { withDefaults } from ".."

const initialState = {
  /*...*/
}

app
  |> withDefaults({ hash: window.location.hash })
  |> {
    init: ({ hash }) => ({ hash, ...initialState }),
    view,
    subscriptions,
    node: document.getElementById("app")
  }

The only people writing subscriptions should know what they’re doing.

People should know what they’re doing whether they’re writing subscriptions or not! 😃

Hence it doesn’t matter if we document the problem with immediate dispatch in subs or not.

A note wouldn’t hurt, but the new documentation I’m working on will be for typical users and getting things done quickly. Implementing your own effects/subscriptions will only make it to the API reference.

jorgebucaran on Jul 6, 2019

@zaceno imo subscriptions are designed for external async changes, dispatching immediate sync actions with subscriptions looks like not appropriate subs api usage (I believe init action is designed for this case). But I’m ok with both sync or async subscriptions (if it brings any real benefit to hyperapp#2 and doesn’t raise once more https://github.com/jorgebucaran/hyperapp/issues/844 upd: anyway docs have to be updated with proper warning/description

sergey-shpak on Jul 5, 2019

A solution (which is maybe incorrect) is to mimick the render deferring for subscriptions, where running the next subscription patch is deferred/locked.

A lock mechanism on patchSubs which prevents infinte looping, but without deferring to next tick, should be possible I think, and would solve the problem for @mrozbarry as well.

By lock mechanism I mean a recursion guard. basically:

if (already entered this function but didn't leave it yet) return

zaceno on Jul 4, 2019

So in conclusion:

Things which require immediate dispatch should be implemented with effects.
Therefore subscriptions should never need to do immediate dispatch
Therefore it doesn’t matter if using immediate dispatch in subscriptions breaks or not
Therefore this issue has no bearing on wether we make subs async or sync.
The only people writing subscriptions should know what they’re doing.
Hence it doesn’t matter if we document the problem with immediate dispatch in subs or not.

Is this correct @jorgebucaran ?

(I may not 100% buy into the assumptions, but I can follow the logic at least.)

zaceno on Jul 6, 2019

@sergey-shpak This issue is about sync/async subscriptions.

@zaceno Rather than dispatching the hashchange action by force, your app should be initialized with the hash state from the start: init: { route: window.location.hash || "#" }.

I believe that is the right approach, the problem is how to reduce boilerplate. One way is with an HOA: https://codepen.io/anon/pen/agayzP.

EDIT: @sergey-shpak beat me to it. 🙌

jorgebucaran on Jul 5, 2019

@mrozbarry some history context: as @jorgebucaran already mentioned I had an issue with deferred subscriptions (https://github.com/jorgebucaran/hyperapp/issues/844) (in my environment deferred based on ‘requestAnimationFrame’ is never called, as result deferred render and subscriptions are never initialized). To solve the issue we decided to move subscriptions outside from deferred render function (it makes a lot sense, because subscriptions should not always depend on render)

Since dispatch and subscriptions are sync now, immediate dispatching from any subscription will create infinite loop - it is by design. My point is hyperapp#2 should not fix such kind of issues, this has to be done by function itself (which creates the loop). Comparable example is simple recursive increment function:

// infinite loop created and stopped by 'inc' function
function inc(val){return val < 10 ? inc(val++) : val}

This means that stopping infinite loop has to be done on the subscription side,

let initialized = false // or any state property
const sub = (dispatch, { action }) => {
  if(!initialized) {
    initialized = true
    dispatch(action)
  }
}

sergey-shpak on Jul 5, 2019