Add end to end test for Suspense

[bz2]

Lack of a Delayed item in the tree appears to be a bug.

Following up from #250 seems Suspense is the likely culprit as it also messes up the tree display components. Start of a test to reproduce/validate behaviour. Passes in current form, need to add some validations that fail.

[changeset-bot]

💥 No Changeset

Latest commit: ed7dcf4

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[marvinhagemeister]

Good catch 👍

[marvinhagemeister]

Took a look why the Suspense component doesn't have the correct name in the elements panel and it's caused by a missing mangling property name in vnode.ts. There is even a comment about that, but it looks like I didn't get around to fix it earlier. Here is a patch that fixes it and displays Suspense components as "Suspense" as one expects it to.

--- a/src/adapter/10/vnode.ts
+++ b/src/adapter/10/vnode.ts
@@ -57,8 +57,13 @@ export function hasDom(x: any): boolean {
  */
 export function isSuspenseVNode(vnode: VNode): boolean {
 	const c = getComponent(vnode) as any;
-	// FIXME: Mangling of `_childDidSuspend` is not stable in Preact
-	return c != null && c._childDidSuspend;
+	return !!(
+		c != null &&
+		(c._childDidSuspend || // < Preact 10.3.0, mangling name was unstable
+			// >= Preact 10.3.0
+			c._pendingSuspensionCount ||
+			c.__u)
+	);
 }
 
 /**
@@ -185,6 +190,11 @@ export function getDisplayName(vnode: VNode, config: RendererConfig10): string {
 					return `${ctx.displayName}.Provider`;
 				}
 			}
+
+			// Suspense
+			if (isSuspenseVNode(vnode)) {
+				return "Suspense";
+			}
 		}
 
 		return type.displayName || type.name || "Anonymous";

[bz2]

@marvinhagemeister Thanks! Had wondered if it was mangling related but not found the right bit of code yet. Your diff does fix the display of Suspense and probably worth landing that and this test as step one regardless, but two bits of follow up:

1. Is the right check c._pendingSuspensionCount >= 0 || c.__u >= 0? Could do !!c.__c but several things are mangled to __c so perhaps there's overlap?

2. The contents of Suspense in the devtools still needs some fixing. Currently it's Suspense > m > Skeleton before the timeout, and Suspense > m after with none of the final content displayed. Need to do a bit more peeking into Suspense internals? Though, it should be props.children that's the final result.

[marvinhagemeister]

@bz2 Agree about landing the change as is. Whenever there is a bit of activity in this repo I tend to roll out releases on a weekly basis and having the proper Suspense name show up is already an improvement worth releasing.

Regarding your questions:

1. Is the right check c._pendingSuspensionCount >= 0 || c.__u >= 0? Could do !!c.__c but several things are mangled to __c so perhaps there's overlap?

Ah good point! I've checked it again and we're save to use __c here. With the mangling naming in Preact we're very attentive to only re-use the same mangling name, if the underlying object is different. You're suggestion to use !!c.__c is the best one in my opinion 👍

2. The contents of Suspense in the devtools still needs some fixing. Currently it's Suspense > m > Skeleton before the timeout, and Suspense > m after with none of the final content displayed. Need to do a bit more peeking into Suspense internals? Though, it should be props.children that's the final result.

I'm a huge fan of splitting work up in chunks. It's totally ok if this PR solves the detection of Suspense and the next one deals with the actual implementation 👍

[marvinhagemeister]

Good news: I've made a branch based of this one and added commits that finish the Suspense integration, se #265. Kept the commit history intact, so the changes in this PR landed with it. Therefore this PR can be closed.

Another PR for toggling suspense state is also incoming 👍

[bz2]

@marvinhagemeister Amazing! Thanks for chasing down.