warn when components yield multiple times in for...of loop

Author: Brian Kim

src/crank.ts

@@ -1438,6 +1438,12 @@ const IsScheduling = 1 << 10;
  */
 const IsSchedulingRefresh = 1 << 11;
 
+/**
+ * A flag which is set when a component has yielded and needs new props. This
+ * flag is used to determine if a component is yielding with stale props.
+ */
+const NeedsNewProps = 1 << 12;
+
 export interface Context extends Crank.Context {}
 
 /**
@@ -1612,10 +1618,12 @@ export class Context<T = any, TResult = any> implements EventTarget {
 					ctx.f |= NeedsToYield;
 				}
 
+				ctx.f &= ~NeedsNewProps;
 				yield ctx.ret.el.props as ComponentProps<T>;
 			}
 		} finally {
 			ctx.f &= ~IsInForOfLoop;
+			ctx.f &= ~NeedsNewProps;
 		}
 	}
 
@@ -2246,7 +2254,7 @@ function enqueueComponentRun<TNode, TResult>(
 		// the bottom of the loop to be reached before returning the inflight
 		// value.
 		const isAtLoopbottom = ctx.f & IsInForAwaitOfLoop && !ctx.onProps;
-		resumePropsIterator(ctx);
+		resumePropsAsyncIterator(ctx);
 		if (isAtLoopbottom) {
 			if (ctx.inflightBlock == null) {
 				ctx.inflightBlock = new Promise(
@@ -2361,7 +2369,7 @@ function runComponent<TNode, TResult>(
 	const ret = ctx.ret;
 	const initial = !ctx.iterator;
 	if (initial) {
-		resumePropsIterator(ctx);
+		resumePropsAsyncIterator(ctx);
 		ctx.f |= IsSyncExecuting;
 		clearEventListeners(ctx);
 		let result: ReturnType<Component>;
@@ -2401,6 +2409,7 @@ function runComponent<TNode, TResult>(
 			];
 		}
 	} else if (hydrationData !== undefined) {
+		// hydration data should only be passed on the initial render
 		throw new Error("Hydration error");
 	}
 
@@ -2438,6 +2447,12 @@ function runComponent<TNode, TResult>(
 			}
 		}
 
+		if (ctx.f & NeedsNewProps && !(ctx.f & IsUnmounted)) {
+			console.error("Component yielded twice without accessing new props");
+		} else if (ctx.f & IsInForOfLoop) {
+			ctx.f |= NeedsNewProps;
+		}
+
 		if (isPromiseLike(iteration)) {
 			throw new Error("Mixed generator component");
 		}
@@ -2466,7 +2481,7 @@ function runComponent<TNode, TResult>(
 		const block = isPromiseLike(value) ? value.catch(NOOP) : undefined;
 		return [block, value];
 	} else if (ctx.f & IsInForOfLoop) {
-		// TODO: does this need to be done async?
+		// Async generator component using for...of loop
 		ctx.f &= ~NeedsToYield;
 		// we are in a for...of loop for async generator
 		if (!initial) {
@@ -2488,6 +2503,12 @@ function runComponent<TNode, TResult>(
 		const block = iteration.catch(NOOP);
 		const value = iteration.then(
 			(iteration) => {
+				if (ctx.f & NeedsNewProps && !(ctx.f & IsUnmounted)) {
+					console.error("Component yielded twice without accessing new props");
+				} else if (ctx.f & IsInForOfLoop) {
+					ctx.f |= NeedsNewProps;
+				}
+
 				let value: Promise<ElementValue<TNode>> | ElementValue<TNode>;
 				if (!(ctx.f & IsInForOfLoop)) {
 					runAsyncGenComponent(ctx, Promise.resolve(iteration), hydrationData);
@@ -2648,7 +2669,7 @@ async function runAsyncGenComponent<TNode, TResult>(
 /**
  * Called to resume the props async iterator for async generator components.
  */
-function resumePropsIterator(ctx: ContextImpl): void {
+function resumePropsAsyncIterator(ctx: ContextImpl): void {
 	if (ctx.onProps) {
 		ctx.onProps(ctx.ret.el.props);
 		ctx.onProps = undefined;
@@ -2728,14 +2749,14 @@ function unmountComponent(ctx: ContextImpl): void {
 			} else {
 				// The logic for unmounting async generator components is in the
 				// runAsyncGenComponent function.
-				resumePropsIterator(ctx);
+				resumePropsAsyncIterator(ctx);
 			}
 		}
 	}
 }
 
 function returnComponent(ctx: ContextImpl): void {
-	resumePropsIterator(ctx);
+	resumePropsAsyncIterator(ctx);
 	if (ctx.iterator && typeof ctx.iterator!.return === "function") {
 		try {
 			ctx.f |= IsSyncExecuting;
@@ -2883,7 +2904,7 @@ function handleChildError<TNode>(
 		throw err;
 	}
 
-	resumePropsIterator(ctx);
+	resumePropsAsyncIterator(ctx);
 	let iteration: ChildrenIteratorResult | Promise<ChildrenIteratorResult>;
 	try {
 		ctx.f |= IsSyncExecuting;

test/warnings.tsx

@@ -3,6 +3,7 @@ import * as Assert from "uvu/assert";
 import * as Sinon from "sinon";
 
 import {createElement} from "../src/crank.js";
+import type {Child, Context} from "../src/crank.js";
 import {renderer} from "../src/dom.js";
 
 const test = suite("warnings");
@@ -40,4 +41,36 @@ test("sync generator component warns on implicit return", async () => {
 	Assert.is(mock.callCount, 1);
 });
 
+test("for of with multiple yields", async () => {
+	function* Component(this: Context): Generator<Child> {
+		for ({} of this) {
+			yield <div>Hello</div>;
+			yield <div>Goodbye</div>;
+		}
+	}
+
+	renderer.render(<Component />, document.body);
+	Assert.is(document.body.innerHTML, "<div>Hello</div>");
+	renderer.render(<Component />, document.body);
+	Assert.is(document.body.innerHTML, "<div>Goodbye</div>");
+	Assert.is(mock.callCount, 1);
+});
+
+test("for of with multiple yields in async generator component", async () => {
+	async function* Component(this: Context): AsyncGenerator<Child> {
+		for ({} of this) {
+			yield <div>Hello</div>;
+			yield <div>Goodbye</div>;
+		}
+	}
+
+	await renderer.render(<Component />, document.body);
+	Assert.is(document.body.innerHTML, "<div>Hello</div>");
+	await new Promise((resolve) => setTimeout(resolve, 100));
+	Assert.is(document.body.innerHTML, "<div>Hello</div>");
+	await renderer.render(<Component />, document.body);
+	Assert.is(document.body.innerHTML, "<div>Goodbye</div>");
+	Assert.is(mock.callCount, 1);
+});
+
 test.run();