Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Virtual DOM for formatters #236

Open
OliverJAsh opened this issue Jul 25, 2014 · 16 comments
Open

Virtual DOM for formatters #236

OliverJAsh opened this issue Jul 25, 2014 · 16 comments

Comments

@OliverJAsh
Copy link
Contributor

Regarding issue #209, I've started to experiment with how we might use a virtual DOM for the formatters in Scribe.

Currently we reset Scribe’s innerHTML every time the formatters are run, which means that we lose all references to nodes inside the Scribe DOM. Some plugins need these references. This is also very bad for performance: innerHTML will cause the layout to be recalculated and re-painted. A virtual DOM would solve both of these problems because we would only ever mutate parts of the Scribe DOM which the formatters make changes to.

Here's what I've got so far: https://gist.github.com/OliverJAsh/cedbd215a8bdf162cf4b

This mimics the curly quotes formatter. I mutate the text nodes, diff my new tree against the current tree, and apply the diff as a patch.

DOM references are kept intact, even for text nodes. I was surprised by this – I thought it would just replace the text node if it's contents are different, but it just updates the textContent/data instead. See the console.

I need a way to clone the current tree (because I don't want to mutate the current tree). I wasn't sure how to do it. Any thoughts? See line 32.

This made me realise, for the formatters to run, we'll have to build a new virtual tree out of the Scribe DOM. We could traverse the actual DOM to re-create an identical virtual DOM?

/cc @theefer @robinedman @hmgibson23

@OliverJAsh
Copy link
Contributor Author

It turns out there's a module to virtualise a real DOM. Updated the gist. https://gist.github.com/OliverJAsh/cedbd215a8bdf162cf4b

@OliverJAsh
Copy link
Contributor Author

The gist now demonstrates how you could use a virtual DOM for the Scribe formatters. Unfortunately I haven't got much further with this, because the virtual-dom library is written in CommonJS which means we are unable to consume it (see jspm/jspm-cli#75 (comment)).

I can't see how we can import this library without moving to jspm – there might be another way.

Me and @theefer were also wondering how we could tidy up the code in the gist. See Matt-Esch/virtual-dom#101.

@OliverJAsh
Copy link
Contributor Author

@theefer @robinedman Just wanted to note that you could taking a look at using the new jspm bundle-sfx: systemjs/builder#2 (comment)

It's not exactly what we want but it might be good enough!

@Raynos
Copy link

Raynos commented Sep 23, 2014

@OliverJAsh

if you want an easier way of importing modules you can use mercury which has a dist/mercury.js file ( https://github.com/Raynos/mercury/blob/master/dist/mercury.js ) with pretty much everything you need.

@mitar
Copy link
Contributor

mitar commented Oct 8, 2014

What is the progress of this? We would need this functionality for exactly the same reason: rendering math equations inside Scribe. Is there a branch to try?

@OliverJAsh
Copy link
Contributor Author

Unfortunately I'm no longer able to maintain Scribe, but AFAIK it's still in heavy use at The Guardian.

@sihil Do you know if any progress has been made on this issue?

@sihil
Copy link
Contributor

sihil commented Oct 9, 2014

Hi @mitar - it's not on our immediate roadmap, but we will get round to it at some point. We've been exploring using a virtual DOM inside one of our newer plugins and are keen to expand that to underpin scribe. Will most likely be a breaking change for all existing plugins in order to keep it simple.

@mitar
Copy link
Contributor

mitar commented Oct 9, 2014

Is there any roadmap on this? Is there any preliminary code on which we could work in parallel?

@sihil
Copy link
Contributor

sihil commented Oct 9, 2014

Aside from the spike that Oliver did there isn't any preliminary code. @robinedman might have some opinions on the best place to start.

@robinedman
Copy link
Contributor

Hi there @mitar. The plugin @sihil mentioned is scribe-plugin-noting.

There's talks about possibly creating a "Scribe v2" that would be based on virtual-dom, maybe the full mercury framework since we might end up using a lot of it anyway. But as @sihil says, it's not on our immediate roadmap.

@bregenspan
Copy link
Contributor

A strategy we're starting to try out at Gawker (not ready yet though) is only formatting nodes that just mutated. This should allow only running formatters on e.g. a paragraph that just changed, and make it possible for formatters to leave things like embedded content such as Iframes or elements with bindings untouched (or at least touched much less frequently). Will update if we find a solution that seems like it makes sense for merge upstream!

@hmgibson23
Copy link
Contributor

This sounds like a good approach. We've been talking about something similar but haven't actually got round to trying it yet. Let us know how it goes!

@bregenspan
Copy link
Contributor

FWIW that approach got quite complicated (each formatter needed both the HTML or element it was modifying AND the surrounding context of the element, and to somehow enforce a pattern of only writing to the element it was operating on and never writing to its surrounding context). So we're actually trying out virtual DOM. Even the following pretty naive and wasteful approach works shockingly well (though not yet ready enough for me to make an upstream PR), and leads to all tests passing:

-            scribe.setHTML(scribe._htmlFormatterFactory.format(scribe.getHTML()));
+            var html = scribe.getHTML();
+            var formattedHtml = scribe._htmlFormatterFactory.format(html);
+            if (html !== formattedHtml) {
+              var vtree = virtualize.fromHTML('<div>' + html + '</div>');
+              var vtree2 = virtualize.fromHTML('<div>' + formattedHtml + '</div>');
+              scribe.el.normalize();  // merge any adjacent text nodes - needed for virtual DOM to apply patches correctly
+              patch(scribe.el, diff(vtree, vtree2));

@hmgibson23
Copy link
Contributor

👍 for this. This looks awesome. I'm looking forward to the upstream PR.

@theefer
Copy link
Contributor

theefer commented Jan 8, 2015

The ultimate goal was to move all plugins to deal with virtual dom rather than real DOM objects, so that's a step in the right direction!

I don't believe running plugins on "diffs" (what mutated) would work as many plugins are highly contextual.

@bregenspan
Copy link
Contributor

@theefer totally agreed -- my goal at the moment is to find a short-term solution that could allow for working with the existing formatters while solving the issue of IFRAMEs refreshing and element references being lost during formatting. I'm a little surprised by the cheapness of transforming HTML to virtual DOM and how well the diff approach works on its own, but it's likely not the ideal long-term solution, which would probably look a lot closer to Medium's implementation, just with some persistent virtual DOM used as the "model".

There are some cases where the HTML->virtual DOM method (which is just https://github.com/marcelklehr/vdom-virtualize ) could use some extra hints to be passed in in order to preserve existing elements in the diff, so looking into those as well as profiling performance.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants