tbranyen/diffhtml

Expose tree creation to middleware

mAAdhaTTah opened this issue · 21 comments

Is it possible to modify the order of the transaction tasks in the middleware? It appears I have access to the array of tasks on the provided transaction, but I'm wondering you think that's bad practice for diffhtml middleware, depending too much on implementation details of the library.

What I'd like to do is "blackbox" some elements, allowing parent components to only update DOM node within their own context and deferring responsibility to children to update themselves. What I was thinking was to insert a task that either modifies the newTree of the transaction before the patches are generated, or modify the patches before they're applied to the DOM. The next step would be updating the HTML-generating function (eventually VDOM-generating...) to only include the HTML required for the parent component. The child would generate their own templates and render themselves on their own.

It appears there are mechanisms to do this; what I'm not sure of is whether that's something I should do. Does diffhtml have assumptions about that sort of thing, or is modifying the transactio before it runs kind of the point?

Since you're modifying the tree, I'd simply add it to run after reconcileTrees. You can see how I inject here: https://github.com/tbranyen/diffhtml/blob/master/packages/diffhtml-components/lib/implementations/react-like.js#L62-L71

Basically, check if reconcileTrees task exists, and if it does, run after it so that you modify the trees immediately after diffHTML and other middleware have completed.

As for best practice, I bet eventually some middleware may collide, but the ones I've written so far all play nicely together. So long as they all run serially and assert their dependencies (throwing an error if the reference task is missing for instance), I think they will be fine.

Unrelated, but is there any way to customize the attribute(s) used for the "key" to determine whether a node is the same between renders?

Currently only key and script#src are valid keys, ref: https://github.com/tbranyen/diffhtml/blob/master/packages/diffhtml/lib/tree/create.js#L145-L148

Although I'm starting to think we need a way to hook into createTree calls w/ the middleware. This would solve your use case by inspecting attributes and setting key if the right attribute is passed. This would also help the diffhtml-components repo to avoid needlessly crawling the tree to find components.

Yeah... that also sounds like it would make the blackboxing easier as well, as I wouldn't have to modify the tree after it's been created.

I've updated the title to reflect this.

Could solve #117 by enabling SVG support as a middleware.

That's a very good idea! Could work like this:

import { use } from 'diffhtml';
import svg from 'diffhtml-middleware-svg';

use(svg(/* optional namespace */));

@mAAdhaTTah I'm thinking something like this added to the 1.0 Middleware Spec

function myMiddleware(options = {}) {
  function myMiddlewareTask() {}

  // Optional: Allows a middleware to hook into the tree creation process.
  //
  // If you wish to modify a VTree immediately after it has been created, you
  // need to specify the nodeType (Number) or nodeName (String) and a function
  // to run that accepts the Virtual Tree. 
  //
  // The value returned from this function becomes the new vTree. So if you
  // plan on only modifying the vTree, return the same instance, this will also
  // be more performant. These hooks are automatically unbound if the
  // middleware is unsubscribed.
  myMiddlewareTask.registerTreeHook(1, function(vTree) {
    /* Do something to the vTree */
    return vTree;
  });

  // Will only execute for divs.
  myMiddlewareTask.registerTreeHook('div', function(vTree) {
    /* Do something to the vTree */
    return vTree;
  });

  // Will only execute for fragments (also shadow dom and components).
  myMiddlewareTask.regsiterTreeHook(11, function(vTree) {
    /* Do something to the vTree */
    return vTree;
  });

  // Shows mirroring `data-some-property` to `key`.
  myMiddlewareTask.regsiterTreeHook(1, function(vTree) {
    if ('data-some-property' in vTree.attributes) {
      vTree.key = vTree.attributes['data-some-property'];
    }

    return vTree;
  });

  return myMiddlewareTask;
}

I dig it so far, and how did I miss that document? Really helpful. Couple questions:

  1. Are the different node types documented yet? Or can we get an enum of them (self-documenting)?
  2. When is this called? If I'm understanding the flow through the rendering process, both the real DOM & (maybe) a string are converted to VDOM, two VDOMs are diffed, patches are generated, and they're applied to the real DOM. So these methods would be called during the real DOM (and maybe string) conversion to VDOM?
  3. Would the given node have its children added already? We could then think of the tree being built from the bottom up, calling these callbacks as it goes.
  4. What other things are you envisioning can be done with this? The hook itself doesn't provide a lot of contextual information (e.g. the transaction in general, where in the DOM tree this node exists, etc.), so your ability to decide what modifications to make is somewhat limited.

Looking forward to playing with it. Also happy to work on this, if you want to set the direction.

@mAAdhaTTah made a PR here with some naive ideas: #123

Some thoughts:

  • You're right, there isn't enough context to make this universally useful (we need to fix that before landing)
  • Should we add hooks for the syncTree stage as well? This could receive both oldTree and newTree and the return value could replace the newTree. I think this might work better for the component's case, since I could easily reuse old instances.

syncTree middleware sounds good too. Can we get access to the actual domNode in the callback as well? Are oldTree or newTree the same instances across calls, e.g. can you cache against them (since that's a pattern I would reuse)?

Good question, if keys are used we could theoretically do that. Which DOM node are you looking for?

The DOM node being patched. If I'm following the flow correct, the syncTree step is where we generate the list of patches, right? Maybe we should provide a hook after that step is finished with the oldTree, newTree, patches, and domNode?

If we pass the transaction object it'll contain all that info. (transaction, oldTree, newTree), think that'd work.

Yeah, although perhaps you wouldn't need a hook then; you could just inject a task that runs after the syncTrees task.

Very true, you could definitely do that and then just pull it off of the transaction object. So I've implemented the syncTreeHook which gets us closer to what you want. The problem currently is that the sync code is pretty complex so there isn't a clear "this is where the hook will have maximum impact" location. The other problem is that I'm not passing along enough information to do key based comparisons for smart rendering.

I'm going to revisit how syncing works to come up with a more middleware-friendly solution. Preferably one that will allow us to break down the sync logic into more discrete functions that leverage the new mechanism.

Might end up being something like a transaction task sub-flow, where functions are flowed the same way, and get passed the oldTree, newTree, and the patchset.

I'll think more on this before we decide to merge anything.

Cool, happy to spitball ideas if you want.

I know this is kind of hacky, but its possible to get access to the transaction object:

export default function myMiddleware(options = {}) {
    let currentTransaction;

    function myTask(transaction) {
        currentTransaction = transaction;

        return () => {
            currentTransaction = undefined;
        };
    };

    // All the hooks now have access to `currentTransaction`.

    return myTask;
}

With that, and access to some of the internals (like NodeCache), I was able to accomplish what I needed in order to make diffhtml work for my use case.

Nice! Glad you were able to get something going. And yeah, while it's hacky now, hopefully you're writing this in such a way that it looks like:

use(myThing());

And once we clean up the underlying middleware APIs you'll be able to easily clean up your middleware without breaking consumer-land code.

Yup, this the middleware. I have it npm link'd on my machine to work on it.

Added in #123