syntax-tree / hast-util-to-dom

utility to transform hast to a DOM tree
https://unifiedjs.com
ISC License
19 stars 8 forks source link

Add visitor function #14

Closed danistefanovic closed 2 years ago

danistefanovic commented 2 years ago

Initial checklist

Problem

Really like the unified ecosystem! I'm currently working on a MutationObserver-based project where those DOM mutations need to be serialized and deserialized. To reconstruct a DOM mutation (e.g., changing the value of a text node), I need to keep track of all DOM nodes that have been deserialized by hast-util-to-dom and connect those to the appropriate hast node and vice-versa. Overview for context:

Record (not relevant for this feature request)

  1. Take a snapshot of the whole DOM and serialize it to a virtual DOM (hast)
  2. MutationObserver detects a text change: listener contains the target DOM node, and with the help of a custom node store, everything gets tracked and serialized

Replay

  1. Deserialize the entire DOM snapshot with hast-util-to-dom
  2. Identify the target DOM node and apply the text change: Unfortunately, this step is not possible with the current hast-util-to-dom functionality because there's no reliable way to map a generated DOM node to a hast node.

Solution

By adding a visitor function, it's possible to inject custom logic to the tree traversal to accomplish the DOM<>VDOM mapping needed in the above example.

options.onNode: (hastNode: HastNode, domNode: Node) => void

Usage example:

function deserialize(
  // A custom store to track all DOM<>VDOM associations
  store: NodeStore,
  // The virtual DOM to deserialize
  vdom: HastNode,
): Node {
  return toDom(vdom, {
    // The new visitor function that receives both nodes as arguments
    onNode: (hastNode: HastNode, domNode: Node) => {
      // Custom code here...
      store.addNode(domNode, hastNode);
    },
  });
}

Although not required for my use case, such a visitor function could also be used in the future similarly to unist-util-visit-parents#visitor to transform nodes while the tree is being traversed instead of looping multiple times through the entire tree.

Alternatives

There's currently no viable alternative. What I've considered for my specific use case:

Happy to provide a PR with the addition!

wooorm commented 2 years ago

Hey!

I understand the use case, and agree that the alternatives are insufficient, though I am not completely sold on the proposed solution yet. Some random thoughts:

danistefanovic commented 2 years ago

This seems like more generic "events" than visitors/tree walking to me

For my use case, definitely yes. However, it depends on whether such a callback should allow tree mutations like unist-util-visit or not.

Does a before counterpart make sense (probably not?)

A "before" callback can make sense to eliminate multiple tree traversals if someone needs to modify the hast before it gets transformed into a DOM node.

import {toDom} from 'hast-util-to-dom';
import {visit} from 'unist-util-visit'

const tree = /* hast with 5000 nodes */

const visitor = (node) => {
  if (node.type === 'element' && node.tagName === 'a') {
    node.properties.href = '#';
  }
}

visit(tree, visitor);
const dom1 = toDom(tree);
// Total of 10'000 interations

const dom2 = toDom(tree, { before: visitor });
// Total of 5000 iterations

Can the handler return a different DOM node?

Good question. If a "before" is introduced, all node mutations could be performed there, and modifying DOM nodes directly should theoretically be unnecessary.

I would go with the minimum: a callback after the hast->DOM transformation without supporting any additional node mutations, but choosing a more generic naming that could be used for all the above cases in the future if needed. Some additional naming ideas:

wooorm commented 2 years ago

I would go with the minimum […] but choosing a more generic naming that could be used for all the above cases in the future if needed

Agreed :+1: I like afterTransform the best. :+1: You were interested in contributing the code, right?

github-actions[bot] commented 2 years ago

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

danistefanovic commented 2 years ago

Will submit a PR within the next day or two!

github-actions[bot] commented 2 years ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.