Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about onStart execution order #111

Closed
ejez opened this issue Jun 29, 2021 · 5 comments
Closed

Question about onStart execution order #111

ejez opened this issue Jun 29, 2021 · 5 comments

Comments

@ejez
Copy link

ejez commented Jun 29, 2021

In the following example I was expecting "onStart" callback to be invoked first, but noticed it was run last:

https://codepen.io/ejez/pen/NWjWBoJ

const src = fromArray([0, 1]);

const { unsubscribe } = pipe(
  src,

  onStart(() => console.log("onStart...")),

  onPush(() => console.log("onPush...")),

  onEnd(() => console.log("onEnd...")),

  subscribe((result) => console.log(result))
);

result:

"onPush..."
0
"onPush..."
1
"onEnd..."
"onStart..."

From the docs:

When the stream starts then the sink is called with Start, Then for every incoming, new value it’s called with Push('a), and when the stream ends it’s finally called with End.

onStart
Run a callback when the Start signal is sent to the sink by the source.

Many thanks.

@kitten
Copy link
Member

kitten commented Jun 30, 2021

The fromArray operator synchronously pushes values using a small trampoline scheduler. It cycles through values as long as values are pulled. All sinks, like subscribe start sending Pull signals as soon as they receive Start.

That leaves the detail of when onStart runs. It actually forwards the Start signal before running the callback, which then means that all values are pushed before this function actually runs in this exceptional case.

The reason for that implementation detail is that if onStart ran before Start was forwarded then we run the risk of executing it before the source has fully started. That's a minor gotcha, but it's currently necessary given the risks of Start not fully propagating, since it's often used as a signal to set up other states.

@ejez
Copy link
Author

ejez commented Jun 30, 2021

@kitten Thanks for the explanation!

I was using onStart to set a fetching state in urql, and was resetting it in onEnd and when a result is received in subscribe. It was working for the first fetch, but breaks when the result is pushed synchronously from cache afterwards, due to the onStart callback executed last. I was not expecting that and it took me a little bit of time to find about the unexpected execution order.

I hope this issue will be of benefit for other users, but an entry in the docs (time permitting) would be appreciated.

Thanks for the hard work on this library!

@kitten
Copy link
Member

kitten commented Jun 30, 2021

Yep, in urql we typically get around this by setting fetching to true before the source is activated/subscribed to. Logically, there are two phases:

  • before the Start signal / before subscription
  • after the Start signal / setup complete

The onStart takes care of the second phase, but the first phase should simply be implemented as synchronous/imperative code before using the source.

@ejez
Copy link
Author

ejez commented Jun 30, 2021

Thanks!

What i did, is adding a condition for setting fetching to true in onSart (only when there is no data, ie first fetch). I will change it as per your suggestion, as it seems onStart is not the natural place for setting the fetching state.

@kitten
Copy link
Member

kitten commented Jun 30, 2021

yea, sounds good 👍 I'll close this for now, since I think the question itself is addressed.

The problems here and ambition is basically always with synchronous streams. The synchronous support in Wonka allows us to apply some nice tricks in urql, but that does come with some caveats. Synchronous events are just very different from asynchronous as it'll always deal with an exact order.

@kitten kitten closed this as completed Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants