foxdonut/meiosis

subtle routing bug

smuemd opened this issue · 6 comments

Hello @foxdonut

I tried out simple-stream with the routing example and ran into an issue.

When entering the app at the initial route path: /beer or /coffee or /login or /settings, I get an Error at the Root component

image

For some reason, state is null at that point.
I logged state one level above in the App component

let count = 0
export class App extends Component {
  /* ... */
  render () {
    const state = this.state
    const { actions } = this.props

    count++
    console.log('App state', state, count)
    return el(Root, { state, actions })
  }
}

and it is in fact null:

image

Now the strange thing is, this happens only when 'cold-loading' the the routing example at the /beer/*, /coffee/*, /settings and /login route paths. The /tea and / paths work when cold loading.

Also this issue only occurs when using simple-stream and mithril/stream. For some reason flyd handles this fine.

I have a hard time wrapping my head around this. Can you reproduce this error?

It must me somehow related to the buffered update mechanism in setup, I think.

Hi @smuemd , I was able to reproduce and I will investigate. Thank you for the bug report!

I suspect it must have something to do with how states(state) may or may not be called in setup script

if (buffer.length > 0) {
// Services produced patches, issue an update and emit the resulting state.
serviceUpdate = true;
update(combine(buffer));
} else {
// No service updates, just emit the resulting state.
buffered = false;
states(state);
}

I also suspect that flyd's atomic updates makes this somehow transparent. But sadly I do not know enough about streams to be really sure. Much less so to suggest a remedy.

Woah, not sure why my commit automatically closed this issue!

In any case, the bug was in setup, where the states stream didn't have an initial value. I tested with simple-stream and mithril/stream, and in both cases I was able to reproduce the bug, and see it resolved after the fix.

Good catch @smuemd !

Ha,

I actually tried, const states = hasServices ? createStream(models()) : models; before submitting the issue. But I missed out on moving these false flags around... Dang.

Do you have any idea why flyd didn't choke on this?

It's working, anyway. (Tested on my machine with the simple-stream / feather / urlon combo)

Do you have any idea why flyd didn't choke on this?

I didn't dig into the details but I did notice some differences between flyd and mithril-stream, I think it has to do with how flyd handles emitting a value onto a stream from within a map'd function of the same stream (or somewhere down the line). In this case, calling update from models.map(...). I noticed a difference where I think it was mithril-stream that calls the function again immediately while flyd finishes the first function's work before triggering the map'd function again.