Fix Suspense-wrapping heuristic (and bump version numbers) #19373

bvaughn · 2020-07-15T15:53:53Z

This PR also includes a package version bump (16.13.1 -> 17.0.0-alpha.0) so that DevTools tests will pass CI.

PR #19108 broke DevTools Suspense integration.

DevTools uses the renderer's version number to figure out what the tag numbers are:

react/packages/react-devtools-shared/src/backend/renderer.js

Lines 156 to 240 in 45eef8b

    
           if (gte(version, '16.6.0-beta.0')) { 
        
             ReactTypeOfWork = { 
        
               Block: 22, 
        
               ClassComponent: 1, 
        
               ContextConsumer: 9, 
        
               ContextProvider: 10, 
        
               CoroutineComponent: -1, // Removed 
        
               CoroutineHandlerPhase: -1, // Removed 
        
               DehydratedSuspenseComponent: 18, // Behind a flag 
        
               ForwardRef: 11, 
        
               Fragment: 7, 
        
               FunctionComponent: 0, 
        
               HostComponent: 5, 
        
               HostPortal: 4, 
        
               HostRoot: 3, 
        
               HostText: 6, 
        
               IncompleteClassComponent: 17, 
        
               IndeterminateComponent: 2, 
        
               LazyComponent: 16, 
        
               MemoComponent: 14, 
        
               Mode: 8, 
        
               OffscreenComponent: 23, // Experimental 
        
               Profiler: 12, 
        
               SimpleMemoComponent: 15, 
        
               SuspenseComponent: 13, 
        
               SuspenseListComponent: 19, // Experimental 
        
               YieldComponent: -1, // Removed 
        
             }; 
        
           } else if (gte(version, '16.4.3-alpha')) { 
        
             ReactTypeOfWork = { 
        
               Block: -1, // Doesn't exist yet 
        
               ClassComponent: 2, 
        
               ContextConsumer: 11, 
        
               ContextProvider: 12, 
        
               CoroutineComponent: -1, // Removed 
        
               CoroutineHandlerPhase: -1, // Removed 
        
               DehydratedSuspenseComponent: -1, // Doesn't exist yet 
        
               ForwardRef: 13, 
        
               Fragment: 9, 
        
               FunctionComponent: 0, 
        
               HostComponent: 7, 
        
               HostPortal: 6, 
        
               HostRoot: 5, 
        
               HostText: 8, 
        
               IncompleteClassComponent: -1, // Doesn't exist yet 
        
               IndeterminateComponent: 4, 
        
               LazyComponent: -1, // Doesn't exist yet 
        
               MemoComponent: -1, // Doesn't exist yet 
        
               Mode: 10, 
        
               OffscreenComponent: -1, // Experimental 
        
               Profiler: 15, 
        
               SimpleMemoComponent: -1, // Doesn't exist yet 
        
               SuspenseComponent: 16, 
        
               SuspenseListComponent: -1, // Doesn't exist yet 
        
               YieldComponent: -1, // Removed 
        
             }; 
        
           } else { 
        
             ReactTypeOfWork = { 
        
               Block: -1, // Doesn't exist yet 
        
               ClassComponent: 2, 
        
               ContextConsumer: 12, 
        
               ContextProvider: 13, 
        
               CoroutineComponent: 7, 
        
               CoroutineHandlerPhase: 8, 
        
               DehydratedSuspenseComponent: -1, // Doesn't exist yet 
        
               ForwardRef: 14, 
        
               Fragment: 10, 
        
               FunctionComponent: 1, 
        
               HostComponent: 5, 
        
               HostPortal: 4, 
        
               HostRoot: 3, 
        
               HostText: 6, 
        
               IncompleteClassComponent: -1, // Doesn't exist yet 
        
               IndeterminateComponent: 0, 
        
               LazyComponent: -1, // Doesn't exist yet 
        
               MemoComponent: -1, // Doesn't exist yet 
        
               Mode: 11, 
        
               OffscreenComponent: -1, // Experimental 
        
               Profiler: 15, 
        
               SimpleMemoComponent: -1, // Doesn't exist yet 
        
               SuspenseComponent: 16, 
        
               SuspenseListComponent: -1, // Doesn't exist yet 
        
               YieldComponent: 9, 
        
             }; 
        
           }

As of #19108, DevTools also decides how to traverse Suspense fibers using the tag values as a form of feature detection:

react/packages/react-devtools-shared/src/backend/renderer.js

Lines 1241 to 1247 in 45eef8b

    
           const areSuspenseChildrenConditionallyWrapped = 
        
             OffscreenComponent === -1; 
        
           if (areSuspenseChildrenConditionallyWrapped) { 
        
             primaryChild = fiber.child; 
        
           } else if (fiber.child !== null) { 
        
             primaryChild = fiber.child.child; 
        
           }

The reason this PR also bumps version numbers is that we previously had multiple Suspense implementations published under 16.13.1 (both the NPM releases and our CI-built releases that tests run against), so the DevTools heuristic failed.

To prevent this type of regression from happening again, a larger effort needs to be made in the form of follow up PRs to get CI running DevTools tests against a range of React releases (not just what's in master).


        Fixed suspense wrapping heuristic


        Bump package numbers 16.13.1 -> 17.0.0-alpha.0 to fix DevTools Suspen…

…se heuristic

bvaughn · 2020-07-15T15:55:29Z

packages/react-devtools-shared/src/__tests__/profilingCache-test.js

@@ -716,4 +716,93 @@ describe('ProfilingCache', () => {
      TestRenderer.create(<Validator commitIndex={0} rootID={rootID} />);
    });
  });
+
+  // See https://github.com/facebook/react/issues/18831
+  it('should not crash during route transitions with Suspense', () => {


This newly added test caught the regression between 16.13.1 (NPM release) and previous DevTools master.

sizebot · 2020-07-15T15:59:42Z

	Warnings
⚠️	Could not find build artifacts for base commit: `4961833`

Size changes (stable)

Generated by 🚫 dangerJS against 2c33b36

sizebot · 2020-07-15T16:00:43Z

	Warnings
⚠️	Could not find build artifacts for base commit: `4961833`

Size changes (experimental)

Generated by 🚫 dangerJS against 2c33b36

bvaughn added 2 commits Jul 15, 2020

Fixed suspense wrapping heuristic

34f1c10

Bump package numbers 16.13.1 -> 17.0.0-alpha.0 to fix DevTools Suspen…

Loading status checks…

2c33b36

…se heuristic

facebook-github-bot added CLA Signed React Core Team labels Jul 15, 2020

bvaughn mentioned this pull request Jul 15, 2020

Fix Suspense-wrapping heuristic #19368

Closed

bvaughn reviewed Jul 15, 2020

View changes

bvaughn requested review from acdlite, gaearon and trueadm Jul 15, 2020

gaearon approved these changes Jul 15, 2020

View changes

facebook / react

Fix Suspense-wrapping heuristic (and bump version numbers) #19373

Fix Suspense-wrapping heuristic (and bump version numbers) #19373

bvaughn commented Jul 15, 2020

This comment has been minimized.

sizebot commented Jul 15, 2020

sizebot commented Jul 15, 2020

facebook / react

Join GitHub today

Fix Suspense-wrapping heuristic (and bump version numbers) #19373

Fix Suspense-wrapping heuristic (and bump version numbers) #19373

Conversation

bvaughn commented Jul 15, 2020

This comment has been minimized.

bvaughn Jul 15, 2020

sizebot commented Jul 15, 2020

Size changes (stable)

sizebot commented Jul 15, 2020

Size changes (experimental)