Fix context propagation for offscreen/fallback trees

[gaearon]

Fixes #19701.

As #19702 shows, this assumption is not always true:

react/packages/react-reconciler/src/ReactFiberNewContext.new.js

Lines 160 to 162 in 9a7e6bf

	// Neither alternate was updated, which means the rest of the
	// ancestor path already has sufficient priority.
	break;

In particular, commenting out this break; fixes #19701.

@acdlite told me this is because inside Offscreen/fallback trees, childLanes may not necessarily be consistent with surroundings because we commit surroundings without the children.

If I just comment out that break; we'll always traverse to the root. This seems bad, especially inside deep trees. Since this function is used during propagation. So I'm changing this to stop at the propagation root.

Aside from fixing what happens inside Offscreen/fallback trees, we also need to consider what happens when the loop goes outside the propagation root. Implementation-wise, this change is not a no-op for nodes outside. You can verify that we used to traverse upwards (past the propagation root) in some cases and modify the childLanes of ancestors. However, @acdlite said that from the model perspective, this shouldn't be necessary to do — we wouldn't have reached the propagation root if there weren't work already scheduled on it. And a parent of a propagation root doesn't need to have dirty childLanes.

So this adds some extra traversal to propagation but buys back some of the cost where we used to do deeper unnecessary traversal upwards past the propagation root. (But now we always stop at the propagation root.) Suspense call from beginWork is an exception though, as it will now always traverse to the root but it used to be able to bail out in the middle. Nvm I fixed that too.

I've included a failing test from #19702 to show it now passes. I imagine there may be other cases we don't know about where context failed to propagate so it's not just about this bug alone. My concern isn't so much Consumer API but the flaw in modeling. The assumption seems fundamentally flawed and I'm worried how else it might bite us.

[sizebot]

Comparing: 9a7e6bf...d465a97

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	129.52 kB	129.53 kB	+0.05%	41.53 kB	41.55 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	134.65 kB	134.66 kB	+0.05%	43.01 kB	43.04 kB
facebook-www/ReactDOM-prod.classic.js	+0.08%	427.76 kB	428.10 kB	+0.06%	78.53 kB	78.58 kB
facebook-www/ReactDOM-prod.modern.js	+0.08%	417.45 kB	417.79 kB	+0.06%	77.06 kB	77.11 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.08%	427.76 kB	428.10 kB	+0.06%	78.53 kB	78.58 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against d465a97

[gaearon]

There is a different fix in #21414. I'm not sure what makes more sense.

[okmttdhr]

I added a comment to make it clear for me. #21414 (comment)

[acdlite]

Mostly nits except for the comment about checking the alternate. I'm curious if one of your test cases broke without that check or if you added it out of caution. If it does break without it, that indicates something else is amiss.

[gaearon]

I'm curious if one of your test cases broke without that check or if you added it out of caution.

No, just in case. I've amended.

[acdlite]

Looks great. Thanks for the thorough test cases!