Why not use the return value of `yield` for prop updates?

[TomerAberbach]

I was wondering if you've considered using the return value of yield for prop updates instead of using for props of this.

I think it would make more sense because passing new arguments to a generator using the next() function is meant for the cooperative multitasking you're trying to achieve.

Additionally, for props of this makes the example in your documentation is a little confusing because the message function parameter is unused and is immediately shadowed by the loop variable:

function *Counter({message}) {
  let count = 0;
  for ({message} of this) {
    count++;
    yield (
      <div>{message} {count}</div>
    );
  }
}

Using the return value of yield for prop updates might be less confusing:

function* Counter({ message }) {
  let count = 0;
  while (true) {
    count++;
    ({ message } = yield (<div>{message} {count}</div>));
  }
}

It would also allow easily implementing different behavior based on prop updates:

function* Counter({ message }) {
  const { message: updatedMessage } = yield (<div>first {message}</div>);

  if (message === updatedMessage) {
    return (<div>Uncreative!</div>);
  }

  message = updatedMessage;

  let count = 1;
  while (true) {
    count++;
    ({ message } = yield (<div>{message} {count}</div>));
  }
}

It also feels more "Just JavaScript" to use the existing generator functionality rather than invent some new concept for cooperative multitasking using for props of this.

I will admit that it's a bit cumbersome to have to parenthesize every object destructuring assignment for each return from yield.

[plusgut]

If I'm not mistaken your approach would also eliminate possible collisions. From my understanding in crank there is always a refresh method inside the this. But if also the props are getting merged into this, that will cause bugs when a prop named refresh is present.

I really like your approach, it looks very clean to me.

[brainkim]

@TomerAberbach Hmm. Out of all the design suggestions so far, this is the one that made me sweat the most 😅. The thing I like about your suggestion is that it forces the invariant that you must yield per each prop update.

However, I think there are several good reason why making this an iterable of props is a better approach:

1. Having to wrap object destructuring reassignments in parens is a real problem. It’s a pain in the butt for what is a very common use-case. for…of over this both provides a block to loop over props and allows you to continue to destructure props in the most ergonomic manner.
2. while (true) is somewhat unsafe, in the sense that you may forget to yield in a specific branch of the loop. Crank contexts provide an invariant to make sure you’re always yielding at least once per iteration. In the future, we may want to add additional invariants, like making sure sync generators yield only once per props iteration.
3. async generators: async generators have a different execution model than sync generators, where new props are received asynchronously, and the loop pauses at the bottom of the for await block for new props. You could pass in promises to yield but this again runs into an ergonomic issue in the sense that you’d have to wrap the yield expression in parens.
4. Having an actual iterable of props is strictly more flexible than passing props back in. It means we can use combinator functions over iterables along with the yield operator (`function Component() { yield *map(this, fn) }`). I think there’s a huge potential for reactive programming with props being iterable/asyncIterable, and if props were just what was passed in we’d miss out on that opportunity.

the message function parameter is unused and is immediately shadowed

Just because a parameter is immediately shadowed does not mean it is unused. Parameters are local variable declarations, and TypeScript will also use the type of props parameter to type JSX.

@plusgut

But if also the props are getting merged into this, that will cause bugs when a prop named refresh is present.

The methods on the context will never clash with props. You’re destructuring the value produced by the iterator, not the context itself.

[plusgut]

@brainkim ah thanks for clarifying

[TomerAberbach]

@brainkim

1. Having to wrap object destructuring reassignments in parens is a real problem. It’s a pain in the butt for what is a very common use-case. for…of over this both provides a block to loop over props and allows you to continue to destructure props in the most ergonomic manner.

I definitely see where you're coming from. It's unfortunate that JS parsers cannot do further look-ahead to disambiguate object destructuring reassignments from blocks. It's particularly unfortunate for those who do not end statements with semicolons (they have to not only wrap object destructuring reassignments in parentheses, but also prefix them with semicolons so that they do not get misinterpreted as function calls over multiple lines).

I don't see a way to get around this other than just accepting it or not destructuring the props in the function parameters like so:

function* Counter(props) {
  let count = 0;
  while (true) {
    const { message } = props 
    count++;
    props = yield (<div>{message} {count}</div>);
  }
}

It's actually not terrible as long as you destructure props at the beginning of your loop.

2. while (true) is somewhat unsafe, in the sense that you may forget to yield in a specific branch of the loop. Crank contexts provide an invariant to make sure you’re always yielding at least once per iteration. In the future, we may want to add additional invariants, like making sure sync generators yield only once per props iteration.

It seems to me that you could just as easily forget to yield in some branch of the for props of this loop, which would mean that you'd accidentally skip over some number of prop updates. Maybe you have some state in your iterable that can detect this and throw an error, which is fine, but that just means that in both the while (true) and for props of this situations you have a non-functioning application with a bug in it; except that in the for props of this case you are now responsible for making sure developers understand what isn't allowed by your framework (that is allowed by JavaScript) and in the while (true) case the developer just has to understand JavaScript while loops and generators. I think the latter is more desirable because it's "Just JavaScript". The invariants are baked into the language features.

3. async generators: async generators have a different execution model than sync generators, where new props are received asynchronously, and the loop pauses at the bottom of the for await block for new props. You could pass in promises to yield but this again runs into an ergonomic issue in the sense that you’d have to wrap the yield expression in parens.

I personally don't think it's too cumbersome to write await (yield expression). I think the fact that we're using async/await and yield means we're willing to put up with some parenthesizing of expressions.

4. Having an actual iterable of props is strictly more flexible than passing props back in. It means we can use combinator functions over iterables along with the yield operator (`function Component() { yield *map(this, fn) }`). I think there’s a huge potential for reactive programming with props being iterable/asyncIterable, and if props were just what was passed in we’d miss out on that opportunity.

Let me know if this code snippet would allay your concerns:

function* map(state, fn) {
  while (true) {
    state = yield fn(state)
  }
}

function* Component(initialState) {
  yield* map(initialState, someFunction)
}

the message function parameter is unused and is immediately shadowed

Just because a parameter is immediately shadowed does not mean it is unused. Parameters are local variable declarations, and TypeScript will also use the type of props parameter to type JSX.

I just meant that in your example it's not immediately clear what purpose (if any) the initial value of message has. I just found it confusing personally.

---

Just want to also say thanks for taking the time to consider my proposal. I think you've come up with a really cool new concept for implementing reactive user interfaces. I'll be a fan regardless of whether you use my ideas or not! :smile:

[monodop]

@TomerAberbach

that just means that in both the while (true) and for props of this situations you have a non-functioning application with a bug in it

True, but only one of those bugs will completely trash your browser.

[brainkim]

I don't see a way to get around this other than just accepting it or not destructuring the props in the function parameters like so

I really like prop destructuring in parameters so I’m hesitant to give this up. I‘ve tried to think about ergonomics, and to focus on, for instance, designing an API which resulted in the fewest keystrokes when converting function components to generator components.

that just means that in both the while (true) and for props of this situations you have a non-functioning application with a bug in it

Like @monodop states, an incorrect while (true) loop will cause your entire page to become unresponsive, while a for...of loop without a yield will throw an error and be localized to that specific render. I don’t think they’re comparable failure states.

for props of this case you are now responsible for making sure developers understand what isn't allowed by your framework (that is allowed by JavaScript)

Here’s another way to think about the advantage of having a separate props iterable. It decouples the concept of generator execution from the concept of responding to props. As I mentioned, these are two different things, especially in the case of async generators, and making them separate calls means that as framework maintainers we can write better diagnostic tools to guide users.

function* map(state, fn) {
  while (true) {
    state = yield fn(state)
  }
}

This would work with the yield* operator, but this is not an idiomatic generator combinator. Typically, the combinators would take an iterable and return a generator, and I haven’t seen any combinator library that didn’t have this sort of API or used the yield expression’s value to do things. Also I want to be able to write code like this in the future:

async function *MessageTime({message} ) {
  let time;
  for await ([{message}, time] of latest(this, createInterval(1000))) {
    yield <div>{message} {time}</div>;
  }
}

This component uses a combinator function to merge the latest values of this with an async iterator which fires a timestamp every second. Isn’t that a cool idea???

[TomerAberbach]

@brainkim

I think you clearly understand my proposal very well and have given it a lot of thought so I'm not going to push for my proposal any further. If you want to keep the design of the framework as is, then sounds good!

Isn’t that a cool idea???

It is a very cool idea!!!

---

One last thing: it might be a crazy idea (I haven't given it much thought), but if you're gonna go with representing prop updates as iterables and async iterables, then maybe let users write components not just as generators, but as any arbitrary function that returns an iterable or async iterable. Otherwise a lot of components might look like:

function* Component(props) {
  yield* someCodeInvolvingPropsAndThis
}

When they could look like:

function Component(props) {
  return someCodeInvolvingPropsAndThis
}

Which doesn't have the unnecessary generator wrapper. Also, if you choose to provide the context as a parameter, then you could use arrow functions like so:

const Component = (context, props) => someCodeInvolvingPropsAndContext

[brainkim]

One last thing: it might be a crazy idea (I haven't given it much thought), but if you're gonna go with representing prop updates as iterables and async iterables, then maybe let users write components not just as generators, but as any arbitrary function that returns an iterable or async iterable.

You can actually write code like:

function MyComponent() {
  return "Hello"[Symbol.iterator]();
}

This component would yield the letters of “Hello” per update and then return when there were no more letters. Under the hood, Crank works by checking to see if an object conforms to the iterator interface to determine if it is a stateful component. Remember that iterables and iterators are separate concepts, and we couldn’t have similar logic for checking if the return value was an iterable, because strings and arrays are iterable, and when you return them from a sync component, they shouldn’t be iterated per component update but all at once.

[TomerAberbach]

I wasn't sure if you were specifically checking for a generator (via value instanceof Generator), an iterable, or an iterator, because generators are all of those things.

Your reasoning for not choosing to check for iterables makes sense. And I'm glad you chose to check for iterators rather than for generators specifically because the former is more flexible.

I'm going to close this issue because I think you've answered the original issue question, but I'm looking forward to seeing where this all goes!

[brainkim]

Happy to have you on the journey! Let me know if you have other thoughts ☺️

[TomerAberbach]

Will do. Thanks!