Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dockview stuck in broken state after api.fromJSON() fails due to invalid component name #341

Closed
mpearson opened this issue Sep 27, 2023 · 3 comments · Fixed by #342
Closed
Labels
bug Something isn't working next release
Milestone

Comments

@mpearson
Copy link
Contributor

Summary
I have a React app which uses local storage to persist the Dockview layout. If there's no layout saved in local storage, it initializes with a default layout. If the saved layout is corrupted (e.g. if it references a panel name that no longer exists) then I want to automatically reset to the default layout. Unfortunately, the Dockview API gets into a broken state in which I can't call api.clear() before rebuilding the default layout. The only way to recover is to delete the local storage entry and reload the page manually.

Reproduction Steps

  1. My layout persistence code is based on this example.
    const savedLayout: SerializedDockview = JSON.parse(
      localStorage.getItem(LOCAL_STORAGE_KEY) || 'null'
    );
    if (savedLayout !== null) {
      try {
        api.fromJSON(savedLayout);
      } catch (err) {
        console.error('Failed to load saved layout:', err);
        localStorage.removeItem(LOCAL_STORAGE_KEY);
        api.clear();
        buildDefaultLayout(api);
      }
    } else {
      buildDefaultLayout(api);
    }
  2. Load the page and allow the default layout to be applied and saved to local storage.
  3. With the default layout, my local storage JSON looks like this:
    {
      "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": "HORIZONTAL"
      },
      "panels": {
        "panelA": { "id": "panelA", "contentComponent": "panelA", "title": "Panel A" },
        "panelB": { "id": "panelB", "contentComponent": "panelB", "title": "Panel B" }
      },
      "activeGroup": "1"
    }
  4. Modify one of the panel components to something invalid, e.g.
    "panelA": { "id": "panelA", "contentComponent": "nonExistentComponent", "title": "Panel A" },
  5. Reload the page
  6. As expected, we see an error raised by the api.fromJSON() call:
    DockLayout.tsx:198 Failed to load saved layout: TypeError: Cannot read properties of undefined (reading 'id')
    
  7. Unfortunately, even if we catch this error, the subsequent call to api.clear() also fails:
    Uncaught Error: Invalid grid element
        at getGridLocation (gridview.ts:111:15)
        at Gridview.remove (gridview.ts:669:26)
        at DockviewComponent.doRemoveGroup (baseComponentGridview.ts:198:36)
        at DockviewComponent.removeGroup (dockviewComponent.ts:704:15)
        at DockviewComponent.clear (dockviewComponent.ts:476:18)
        at DockviewApi.clear (component.api.ts:471:24)
        at buildDefaultLayout (DockLayout.tsx:126:7)
        at DockLayout.tsx:200:9
        at commitHookEffectListMount (react-dom.development.js:23150:26)
        at commitPassiveMountOnFiber (react-dom.development.js:24926:13)
    

Expected behavior
Invalid component names are probably the single most likely form of layout corruption so I think this is an important case to handle cleanly. It makes sense for api.fromJSON() to raise an error, but the layout should be in a clean (empty) state after it's caught.

Environment

  • Chrome 117
  • React 18.2.0
  • Dockview 1.7.6
@mathuo mathuo linked a pull request Sep 28, 2023 that will close this issue
@mathuo
Copy link
Owner

mathuo commented Sep 28, 2023

Thanks for the details! I can confirm I see the same problem.

I agree that the library should be able to recover after being provided with a corrupted layout and currently the dock enters a corrupted state if passed a corrupt layout which even calling .clear() is unable to rectify.

I will change this so that if fromJSON(...) does fail it automatically clears it's state down to an empty grid before throwing back the Error.

@mathuo mathuo added bug Something isn't working next release labels Sep 29, 2023
@mathuo mathuo added this to the v1.8.4 milestone Sep 30, 2023
@mathuo mathuo reopened this Oct 1, 2023
@mathuo
Copy link
Owner

mathuo commented Oct 6, 2023

Released in version 1.8.4

@mathuo mathuo closed this as completed Oct 6, 2023
@mpearson
Copy link
Contributor Author

mpearson commented Oct 8, 2023

Thanks for fixing this man!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants