@samitouri / QOS-React / commits / 358e63ac3d

Fixed a Suspense toggling bug that incorrectly impacted tree weight

Brian Vaughn committed Apr 18, 2019 at 09:38 UTC 358e63ac3d103ca4698fb36777d9278150a6fb37
3 files changed +76 -3
src/__tests__/__snapshots__/store-test.js.snap
+26
@@ -214,6 +214,32 @@ exports[`Store collapseNodesByDefault:true should filter DOM nodes from the stor
214 ▸ <Parent>
215 `;
216
217 +exports[`Store collapseNodesByDefault:true should not add new nodes when suspense is toggled: 1: mount 1`] = `
218 +[root]
219 + ▸ <SuspenseTree>
220 +`;
221 +
222 +exports[`Store collapseNodesByDefault:true should not add new nodes when suspense is toggled: 2: expand tree 1`] = `
223 +[root]
224 + ▾ <SuspenseTree>
225 + ▾ <Suspense>
226 + ▸ <Parent>
227 +`;
228 +
229 +exports[`Store collapseNodesByDefault:true should not add new nodes when suspense is toggled: 3: toggle fallback on 1`] = `
230 +[root]
231 + ▾ <SuspenseTree>
232 + ▾ <Suspense>
233 + <Fallback>
234 +`;
235 +
236 +exports[`Store collapseNodesByDefault:true should not add new nodes when suspense is toggled: 4: toggle fallback on 1`] = `
237 +[root]
238 + ▾ <SuspenseTree>
239 + ▾ <Suspense>
240 + ▸ <Parent>
241 +`;
242 +
243 exports[`Store collapseNodesByDefault:true should support expanding deep parts of the tree: 1: mount 1`] = `
244 [root]
245 ▸ <Wrapper>
src/__tests__/store-test.js
+48 -1
@@ -4,6 +4,7 @@ describe('Store', () => {
4 let React;
5 let ReactDOM;
6 let TestUtils;
7 + let agent;
8 let store;
9
10 const act = (callback: Function) => {
@@ -14,6 +15,7 @@ describe('Store', () => {
15 };
16
17 beforeEach(() => {
18 + agent = global.agent;
19 store = global.store;
20
21 React = require('react');
@@ -409,7 +411,7 @@ describe('Store', () => {
411 );
412 expect(store).toMatchSnapshot('1: mount');
413
412 - const deepestedNodeID = global.agent.getIDForNode(ref.current);
414 + const deepestedNodeID = agent.getIDForNode(ref.current);
415
416 act(() => store.toggleIsCollapsed(deepestedNodeID, false));
417 expect(store).toMatchSnapshot('2: expand deepest node');
@@ -460,5 +462,50 @@ describe('Store', () => {
462 act(() => store.toggleIsCollapsed(store.getElementIDAtIndex(0), true));
463 expect(store).toMatchSnapshot('6: collapse root');
464 });
465 +
466 + it('should not add new nodes when suspense is toggled', () => {
467 + const SuspenseTree = () => {
468 + return (
469 + <React.Suspense fallback={<Fallback>Loading outer</Fallback>}>
470 + <Parent />
471 + </React.Suspense>
472 + );
473 + };
474 +
475 + const Fallback = () => null;
476 + const Parent = () => <Child />;
477 + const Child = () => null;
478 +
479 + act(() =>
480 + ReactDOM.render(<SuspenseTree />, document.createElement('div'))
481 + );
482 + expect(store).toMatchSnapshot('1: mount');
483 +
484 + act(() => store.toggleIsCollapsed(store.getElementIDAtIndex(0), false));
485 + act(() => store.toggleIsCollapsed(store.getElementIDAtIndex(1), false));
486 + expect(store).toMatchSnapshot('2: expand tree');
487 +
488 + // HACK There's only one renderer for this test
489 + const rendererID = Object.keys(agent._rendererInterfaces)[0];
490 + const suspenseID = store.getElementIDAtIndex(1);
491 +
492 + act(() =>
493 + agent.overrideSuspense({
494 + id: suspenseID,
495 + rendererID,
496 + forceFallback: true,
497 + })
498 + );
499 + expect(store).toMatchSnapshot('3: toggle fallback on');
500 +
501 + act(() =>
502 + agent.overrideSuspense({
503 + id: suspenseID,
504 + rendererID,
505 + forceFallback: false,
506 + })
507 + );
508 + expect(store).toMatchSnapshot('4: toggle fallback on');
509 + });
510 });
511 });
src/devtools/store.js
+2 -2
@@ -551,7 +551,7 @@ export default class Store extends EventEmitter {
551
552 if (__DEBUG__) {
553 console.groupCollapsed('onBridgeOperations');
554 - debug('onBridgeOperations', operations);
554 + debug('onBridgeOperations', operations.join(','));
555 }
556
557 let haveRootsChanged = false;
@@ -772,7 +772,7 @@ export default class Store extends EventEmitter {
772
773 children.forEach(childID => {
774 const child = ((this._idToElement.get(childID): any): Element);
775 - childWeight += child.weight;
775 + childWeight += child.isCollapsed ? 1 : child.weight;
776 });
777
778 element.weight = childWeight + 1;