Fixed bug in Store.getIndexOfElementID() that caused roots with multiple top-level children to return an incorrect item index
Brian Vaughn committed
May 2, 2019 at 15:12 UTC
d7447a8458c8196425e94ef746ba6c9d1be6f421
2 files changed
+94
-6
src/__tests__/store-test.js
+87
@@ -696,4 +696,91 @@ describe('Store', () => {
696
expect(store).toMatchSnapshot('4: toggle fallback on');
697
});
698
});
699
+
700
+ describe('getIndexOfElementID', () => {
701
+ beforeEach(() => {
702
+ store.collapseNodesByDefault = false;
703
+ });
704
+
705
+ it('should support a single root with a single child', () => {
706
+ const Grandparent = () => (
707
+ <React.Fragment>
708
+ <Parent />
709
+ <Parent />
710
+ </React.Fragment>
711
+ );
712
+ const Parent = () => <Child />;
713
+ const Child = () => null;
714
+
715
+ act(() =>
716
+ ReactDOM.render(<Grandparent />, document.createElement('div'))
717
+ );
718
+
719
+ for (let i = 0; i < store.numElements; i++) {
720
+ expect(store.getIndexOfElementID(store.getElementIDAtIndex(i))).toBe(i);
721
+ }
722
+ });
723
+
724
+ it('should support multiple roots with one children each', () => {
725
+ const Grandparent = () => <Parent />;
726
+ const Parent = () => <Child />;
727
+ const Child = () => null;
728
+
729
+ act(() => {
730
+ ReactDOM.render(<Grandparent />, document.createElement('div'));
731
+ ReactDOM.render(<Grandparent />, document.createElement('div'));
732
+ });
733
+
734
+ for (let i = 0; i < store.numElements; i++) {
735
+ expect(store.getIndexOfElementID(store.getElementIDAtIndex(i))).toBe(i);
736
+ }
737
+ });
738
+
739
+ it('should support a single root with multiple top level children', () => {
740
+ const Grandparent = () => <Parent />;
741
+ const Parent = () => <Child />;
742
+ const Child = () => null;
743
+
744
+ act(() =>
745
+ ReactDOM.render(
746
+ <React.Fragment>
747
+ <Grandparent />
748
+ <Grandparent />
749
+ </React.Fragment>,
750
+ document.createElement('div')
751
+ )
752
+ );
753
+
754
+ for (let i = 0; i < store.numElements; i++) {
755
+ expect(store.getIndexOfElementID(store.getElementIDAtIndex(i))).toBe(i);
756
+ }
757
+ });
758
+
759
+ it('should support multiple roots with multiple top level children', () => {
760
+ const Grandparent = () => <Parent />;
761
+ const Parent = () => <Child />;
762
+ const Child = () => null;
763
+
764
+ act(() => {
765
+ ReactDOM.render(
766
+ <React.Fragment>
767
+ <Grandparent />
768
+ <Grandparent />
769
+ </React.Fragment>,
770
+ document.createElement('div')
771
+ );
772
+ ReactDOM.render(
773
+ <React.Fragment>
774
+ <Grandparent />
775
+ <Grandparent />
776
+ </React.Fragment>,
777
+ document.createElement('div')
778
+ );
779
+ });
780
+
781
+ for (let i = 0; i < store.numElements; i++) {
782
+ expect(store.getIndexOfElementID(store.getElementIDAtIndex(i))).toBe(i);
783
+ }
784
+ });
785
+ });
786
});
src/devtools/store.js
+7
-6
@@ -429,12 +429,6 @@ export default class Store extends EventEmitter {
429
let index = 0;
430
while (true) {
431
const current = ((this._idToElement.get(currentID): any): Element);
432
- if (current.parentID === 0) {
433
- // We found the root; stop crawling.
434
- break;
435
- }
436
-
437
- index++;
432
433
const { children } = current;
434
for (let i = 0; i < children.length; i++) {
@@ -446,6 +440,13 @@ export default class Store extends EventEmitter {
440
index += child.isCollapsed ? 1 : child.weight;
441
}
442
443
+ if (current.parentID === 0) {
444
+ // We found the root; stop crawling.
445
+ break;
446
+ }
447
+
448
+ index++;
449
+
450
previousID = current.id;
451
currentID = current.parentID;
452
}