binaryage / cljs-devtools

A collection of Chrome DevTools enhancements for ClojureScript developers
Other
1.11k stars 51 forks source link

Annotating display elements with path information #56

Closed mike-thompson-day8 closed 3 years ago

mike-thompson-day8 commented 4 years ago

We use clj-devtools within re-frame-10x. We use clj-devtools to render CLJS data to browse-able HTML and let the programmer inspect it. And it is really good, thanks!

(So, just to be crystal clear, our use is outside of Chrome devtools - we're using this library to render CLJS data structures in a normal HTML page setting)

When a user of re-frame10x is browsing around the data structures, the user would like to be able to right-click on an arbitrary node (within the rendering) and obtain information about that node, specifically the path from the root of the data structure to the point they are clicking.

So, by right-clicking on some value in the rendering, they might get a popup menu and choose the "path to clipboard" option, which might put a string like this into the clipboard:

[:a :path :to 5 :the :item]    ;; 5 is meant to indicate some index into a vector 

Now, obviously this is not possible currently. And previously when this has come up we've just said "no" can't be done, end of story. But it came up again recently and in the process of saying "no", I had a further thought ...

Could this be achieved using the following technique:

  1. during the render process ... decorate each HTML element with an attribute, called say data-clj-path, which would carry the [:a :path :to 5 :the :item] information as a string.
  2. then, in the context of re-frame-10x, we could wrap a right-click handler around our use of clj-devtools and, in the handler, inspect the source HTML element for its data-clj-path. Bingo, now we have the path and can write it to the clipboard ... or whatever else).

I'd be prepared to have a go at making this happen, but before I take on the learning curve, I wanted to check in with you:

  1. off the top of your head, does this seem possible to you? Ie. can the current path be rendered into the HTML as a data-* attribute? Or should I just stop right now and abandon all hope. :-)
  2. Would you be willing to accept a PR for this, assuming I can find some time to do it?
  3. (optional) if you would be willing to accept a PR, do you have any hints about where I should look first?
darwin commented 4 years ago

Hi Mike, I'm aware of your JSON-ML renderer. I'm confident that we will figure some solution out and I don't think it will be that much work. I'm willing to accept a PR.

Let me give you some context and suggestions.

markup

Internally cljs-devtools renders hiccup-like markup (pure cljs data) which gets turned into JSON-ML as final step (produces JSON with JS arrays and stuff suitable as an input for devtools custom formatters).

The render-markup fn is responsible for turning markup data into JSON-ML. Various templating functions used for producing markup data can be seen in markup.cljs. They produce nested vectors with lots of keywords, which get resolved into real values (js strings and js style objects) via prefs during render-markup.

In reframe-10x you are using higher-level api which uses render-markup.

printing

Ok, we know how to render JSON-ML from markup data. And we have a library of template functions for producing markup data. But how we turn given arbitraily complex cljs value into markup data?

The answer is quite complicated because we don't want to re-implement cljs printing in cljs-devtools. We want to hook into clojurescript printing subsystem. The idea is that we let cljs print the value and let it call us back when printer recursively walks the structure and wants to print sub-values. I call this "managed printing" and it is implemented here, for full rationale see CLJS-1010.

state

JSON-ML and cljs-devtools do not show everything at once. They render stuff incrementally as user discloses various parts of the data structure. This complicates some things because we have to carry some state over. Custom formatters luckily give us mechanism to carry over arbitrary state (via their "config") and this allows use resume rendering from last point. Please review formatters/state.cljs there is described how relevant "resumed" state is dynamically bound during rendering a markup.

look at detection of circular data

You seem to be lucky. I had to internally implement tracking current "path" of drilled down cljs objects because I wanted to detect and visually mark drilling down into circular data structures. And for this I keep "history" of visited objects on the rendered path.

When you search code for push-object-to-current-history! and is-circular? usages you end up in alt-printer-impl and I believe this is exactly the place where you want to implement your path functionality. I think you might be able just use get-current-history as is.

my suggestions

  1. introduce a new pref which will be a feature-flag for enabling this functionality, enable it in re-frame10x, this way you will have flexibility to freely enhance JSOM-ML for re-frame10x's JSON-ML renderer, without breaking devtools custom formatters assumptions.

  2. emit the path data into JSON-ML in a suitable form, it looks to me that adding it as a new post-processing step to post-process-printed-output would be the best place. I would prefer to keep alt-printer-implas simple as possible.

Gregg8 commented 4 years ago

Hi Antonin,

Mike asked me to work on this PR. Working through your very helpful info and suggestions:

However, when I started looking at how to insert the data-clj-path as a new attribute, it appears that would require quite a bit of refactoring, unless of course I am mistaken (possibly, hopefully).

It looks like the hiccup-like markup only supports styles and has no way of representing other attributes.

Here's a simple example of some markup passed into post-process-printed-output:

([:keyword-tag ":id"])

How could I insert a data-* attribute in this?

Further, the primary function which outputs the markup to JsonML make-template takes tag, style & children.

As you know, the style doesn't come in as a map (so we can't easily attach an extra attribute payload), but as a string of fully-formed CSS.

Without testing, I have re-written it to accept an attributes map:

(defn make-attributes-template
  [tag attributes & children]
  (let [tag (pref tag)
        template (mark-as-template! #js [tag (clj->js attributes)])]
    (doseq [child children]
      (if (some? child)
        (if (coll? child)
          (.apply (unchecked-aget template "push") template (mark-as-template! (into-array (keep pref child)))) collections
          (if-let [child-value (pref child)]
            (.push template child-value)))))
    template))

Before I proceed any further, I need your thoughts on the best way to add attributes to the markup to minimise the impact on the current code.

Also, I haven't worked out the best way to calculate the actual path in the middle of the post-process-printed-output function, but I'll start looking at that now.

Gregg8 commented 4 years ago

So whereas style looks something like this:

"background-color:rgba(100,255,100,0.08);color:rgba(0,0,0,1);border-radius:2px;"

attributes would look something like this:

 {:style         "background-color:rgba(100,255,100,0.08);color:rgba(0,0,0,1);border-radius:2px;"
  :data-clj-path "[:a :path :to 5 :the :item]"}
darwin commented 4 years ago

I had to look into it a bit deeper and play with the code as well. I haven't really touched this code since I wrote it in 2017-ish, so I don't have full context in my head.

render-markup does prefs expansion and this could happen multiple times until we get to "raw values". This is for convenience for writing code in markup.cljs in more abstract way, e.g. one can emit markup like [:some-tag child1 child2 ...]. Assume this in prefs:

{
  :some-tag [:span :some-style]
  :span "span"
  :some-style "color:red;"
   ...
}

Render markup will expand [:some-tag child1 child2] to: [[:span :some-style] child1 child2] and this is processed by render-subtree which calls make-template with :span, :some-style, rendered-child1 and rendered-child2. And make-template further expands :span and :some-style and returns final JSON (plain js value):

["span" {"style" "color:red;"} ~rendered-child1 ~rendered-child2]

Also now I realize that post-process-printed-output gets "group" on input. Which is just a soup of markups and strings. Not guaranteed to have a tag keyword as the first item. There could be some strings intermixed as well. So it is not straightforward for you to pass something extra there.

At this point I think easiest would be to add a new wrapper tag to annotate subtrees with path-info, e.g. JSONML would be

["annotation" {:path ":some :path"} original-group...]

I will prepare a proof of concept code. Hopefully tomorrow.

darwin commented 4 years ago

@Gregg8 please look at the issue-56 branch. I think the basic idea is there, you will probably want to make building paths more robust and suitable for your use cases. I just hacked quick support for maps and vectors. Not all subtrees will have presentable paths. Also note that objects could be pure javascript objects which have nothing to do with cljs.

Gregg8 commented 4 years ago

@darwin many thanks for this proof of concept branch. I did spend time understanding the internals of cljs-devtools and working in 10x to connect it all up and got it working to a degree. However, it took me much more time than I had hoped and there was still potentially a decent amount of work left to do (to get correct paths for all object types etc.), so I had to get back to other Day8 projects. I have been intending to work in my own time and that also hasn't exactly worked out either, hence this late reply. When I can find time to get back to this, I will as I believe it will be a really useful feature. Thanks again for your assistance.

MawiraIke commented 3 years ago

Hi @darwin . Mike asked me to proceed on this issue. After checking the proof of concept branch, I have managed to make changes that would improve path generation in devtools. My changes are on PR #62 . Thank you for the detailed instructions and for the proof of concept branch, they helped a lot. I have included comments in my changes to highlight on the idea/why the change is suggested. I hope these changes are acceptable. I will be available to make any further changes or to explain the commit. Again, thank you for the help.

darwin commented 3 years ago

Merged in v1.0.4. Thanks for your cooperation.