org-roam-ui icon indicating copy to clipboard operation
org-roam-ui copied to clipboard

Define / clarify behaviors when sqlite databases are empty

Open jhhb opened this issue 2 years ago • 1 comments

Thanks for your time and help and for the excellent work on this project!

Expected

  • When running org-roam-ui-mode, when the sqlite databases are empty (this occurs when the directory specified byorg-roam-directory contains no org roam files), the UI renders without any visible frontend errors or console errors.

Actual

  • In the browser console, I see an error:
Uncaught TypeError: e.nodes is null
  • I receive this websocket data:
{"type":"graphdata","data":{"nodes":null,"links":null,"tags":null}}

I think this is something we could fix when serializing the data that we send via the websocket by ensuring that all alist entries have a non-nil value

Desired behavior

  • The UI works with an empty DB / empty directory without any kind of error to reduce friction for brand new users
  • In a perfect world, there might even be a UI notice / alert to the user that they have no notes created yet. IMO, this is outside of the scope here, but I'd be happy to work on this if anyone is interested.

Suggested change

Thanks to @kirillrogovoy for suggesting a backend change as a good path. I'm happy to contribute any change; below is my thinking from looking at the code a bit:

Ideally, we prevent this state from ever occurring via Elisp changes. When we have an empty sqlite db, this is the value of the response variable at https://github.com/org-roam/org-roam-ui/blob/b153f4fee99e36dec0fb56d987026d53bf97a0e8/org-roam-ui.el#L321 :

; response variable has this structure before we send it to the server when dbs are empty:
'((nodes) (links) (tags))
; this serializes like this:
(json-encode '((nodes) (links) (tags))
"{\"nodes\":null,\"links\":null,\"tags\":null}"

I was thinking we might be able to fix this issue by mapping over the response alist entries and sanitizing so that in the above example, we end up encoding this structure, instead of the original structure where nulls are possible:

(json-encode '((nodes . []) (links . []) (tags . [])))
"{\"nodes\":[],\"links\":[],\"tags\":[]}"

Repro steps

  1. Run the following Elisp snippet, which creates an empty directory, and then follows suggested org-roam setup and org-roam-ui setup, binding org-roam-directory to the empty directory.
(defun repro()
  ; org-roam
  (require 'org-roam)

  (let ((real-dir "~/Desktop/notes/org-roam") (empty-dir "~/Desktop/empty-dir"))
    (make-directory empty-dir t)
    (setq org-roam-directory empty-dir))
  (setq org-roam-v2-ack t)
  (org-roam-setup)

  ; org-roam-ui
  (add-to-list 'load-path "~/code/org-roam-ui")
  (load-library "org-roam-ui")
)
(repro)
  1. Run (org-roam-ui-mode) and navigate to the opened window.
  2. See a blank screen with a console error at http://localhost:35901/

Related issues or PRs

  • This is a similar type of issue as #17 which happens when links are null
  • https://github.com/jhhb/org-roam-ui/pull/1#discussion_r686509765

jhhb avatar Aug 11 '21 18:08 jhhb

Hey!

Thanks for your effort writing all the up!

I think the main obstacle here is that, for Elisp, an empty array and nil are essentially the same thing, so it does make sense for json-encode to turn it into null.

Now, for us JS devs it's not what we want or even expect.

Looking at the code, it feels to me like there's no elegant solution for that on the Elisp side. At the same time, I'm intuitively inclined towards some blatant in-your-face condition so that the behavior is very visible. But it's hard to come up with any.

Maybe we should encode "nodes", "links", and "tags" separately with some new stupid function (e.g. json-encode-or-empty-array) that would produce [] when the input is nil-ish, otherwise, just call json-encode. And then we manually join them to produce a valid JSON string before sending it to the browser.

It's ugly AF but at least it's stupid in a good kind of way. :)

Or maybe there's a simpler solution which I don't know because I actually suck at Elisp.

kirillrogovoy avatar Aug 12 '21 09:08 kirillrogovoy