bug: unable to recover from loading a corrupted layout

mathuo · Sep 28, 2023 · 8081c92 · 8081c92
1 parent 4ad5b0f
commit 8081c92
Show file tree

Hide file tree

Showing 2 changed files with 126 additions and 21 deletions.
diff --git a/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts b/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts
@@ -4063,4 +4063,77 @@ describe('dockviewComponent', () => {
         expect(tabDragEvents.length).toBe(0);
         expect(groupDragEvents.length).toBe(1);
     });
+
+    test('loading a corrupted layout', () => {
+        const container = document.createElement('div');
+
+        const dockview = new DockviewComponent({
+            parentElement: container,
+            components: {
+                default: PanelContentPartTest,
+            },
+            tabComponents: {
+                test_tab_id: PanelTabPartTest,
+            },
+            orientation: Orientation.HORIZONTAL,
+        });
+
+        dockview.layout(1000, 500);
+
+        dockview.addPanel({
+            id: 'panel_1',
+            component: 'default',
+        });
+
+        expect(dockview.groups.length).toBe(1);
+        expect(dockview.panels.length).toBe(1);
+
+        dockview.fromJSON({
+            grid: {
+                root: {
+                    type: 'branch',
+                    data: [
+                        {
+                            type: 'leaf',
+                            data: {
+                                views: ['panelA'],
+                                activeView: 'panelA',
+                                id: '1',
+                            },
+                            size: 841,
+                        },
+                        {
+                            type: 'leaf',
+                            data: {
+                                views: ['panelB'],
+                                activeView: 'panelB',
+                                id: '2',
+                            },
+                            size: 842,
+                        },
+                    ],
+                    size: 530,
+                },
+                width: 1683,
+                height: 530,
+                orientation: Orientation.HORIZONTAL,
+            },
+            panels: {
+                panelA: {
+                    id: 'panelA',
+                    contentComponent: 'somethingBad',
+                    title: 'Panel A',
+                },
+                panelB: {
+                    id: 'panelB',
+                    contentComponent: 'panelB',
+                    title: 'Panel B',
+                },
+            },
+            activeGroup: '1',
+        });
+
+        expect(dockview.groups.length).toBe(0);
+        expect(dockview.panels.length).toBe(0);
+    });
 });
diff --git a/packages/dockview-core/src/dockview/dockviewComponent.ts b/packages/dockview-core/src/dockview/dockviewComponent.ts
@@ -649,33 +649,65 @@ export class DockviewComponent
         const createGroupFromSerializedState = (data: GroupPanelViewState) => {
             const { id, locked, hideHeader, views, activeView } = data;
 
-            const group = this.createGroup({
-                id,
-                locked: !!locked,
-                hideHeader: !!hideHeader,
-            });
+            if (typeof id !== 'string') {
+                throw new Error('group id must be of type string');
+            }
 
-            this._onDidAddGroup.fire(group);
+            let group: DockviewGroupPanel | undefined;
 
-            for (const child of views) {
-                const panel = this._deserializer.fromJSON(panels[child], group);
+            try {
+                group = this.createGroup({
+                    id,
+                    locked: !!locked,
+                    hideHeader: !!hideHeader,
+                });
 
-                const isActive =
-                    typeof activeView === 'string' && activeView === panel.id;
+                this._onDidAddGroup.fire(group);
 
-                group.model.openPanel(panel, {
-                    skipSetPanelActive: !isActive,
-                    skipSetGroupActive: true,
-                });
-            }
+                for (const child of views) {
+                    const panel = this._deserializer.fromJSON(
+                        panels[child],
+                        group
+                    );
 
-            if (!group.activePanel && group.panels.length > 0) {
-                group.model.openPanel(group.panels[group.panels.length - 1], {
-                    skipSetGroupActive: true,
-                });
-            }
+                    const isActive =
+                        typeof activeView === 'string' &&
+                        activeView === panel.id;
 
-            return group;
+                    group.model.openPanel(panel, {
+                        skipSetPanelActive: !isActive,
+                        skipSetGroupActive: true,
+                    });
+                }
+
+                if (!group.activePanel && group.panels.length > 0) {
+                    group.model.openPanel(
+                        group.panels[group.panels.length - 1],
+                        {
+                            skipSetGroupActive: true,
+                        }
+                    );
+                }
+
+                return group;
+            } catch (err) {
+                /**
+                 * This is an odd case... we have failed to deserialize a view but we have already created a group,
+                 * but we havn't registered that group with the gridview.
+                 * We cannot use the removeGroup method because the group has only been partially added, we must
+                 * manually dipose() of the view and remove it from being stored in the map.
+                 */
+                if (group) {
+                    group.dispose();
+                    this._groups.delete(group.id);
+                }
+
+                /**
+                 * re-throw the error becasue we don't actually want to catch it, we just
+                 * needed to do some clean-up before continuing.
+                 */
+                throw err;
+            }
         };
 
         this.gridview.deserialize(grid, {