Support for async visitors

[tomasbonco]

In my opinion, this should be working:

await visit( tree, 'code', visitor )

function visitor( node, index, parent ) {
    return new Promise( resolve => {
        const removeNode = () => {
            parent.children.splice( index, 1 );
            resolve()
        }
		
        setTimeout( removeNode, 10 );
    })
}

Another (more abstract) example:

await visit( tree, 'code', visitor )

async function visitor( node, index, parent ) {
   node.value = await someAsyncStuff();
}

Background

Visitor might do:

1. read a node
2. perform an action based on the node value/props (reading/writing files, do a HTTP request, ...)
3. modify the tree based on the result of async operation

However, currently, only sync operations are allowed. That's why it cannot be used for async transformers, which should be supported by unified.

[wooorm]

Can you go a level higher? What do you practically want to do?

[tomasbonco]

I'm doing a live preview. I find code nodes, do the compilation, save the content to the file, then replace the code node for a custom node (transforms into a <script> tag) pointing to the file. Then I parse it with rehype.
Yes, currently it does "work" because I return a future filename immediately, but I'm unable to catch errors and propagate them "higher".

[tomasbonco]

I was thinking about it just a little bit more and a workaround could look like this:

async function transform( tree ) {
    const matches = [];

    visit( tree, 'code', visitor )

    function visitor( node, index, parent ) {
        matches.push({ node, index, parent })
    }

    const promises = matches.map( async match => {
        const filename = await someAsyncStuff( match.node.value )
        // ... modify tree ...
    })

    await Promise.all( promises );
    return tree;
}

But I'm not sure, how exactly does visit work, because this could cause issues with removing notes (indexes might get out of sync). But it might be a case anyway.

[wooorm]

I think your new example is indeed (almost) the way to go: otherwise, you would do a waterfall. You’d only be able to process serially, but with that new approach, you can send out all requests immediately!

The point about the indexes is true! But you can work around that: you need the parent, and the node. And then you can do parent.children[parent.children.indexOf(node)] = replacement!

[tomasbonco]

Of course one can do indexOf oneself. I was thinking about the drawbacks of the solution ... whether the request is still valid. Now I think a wrapper function is more suitable. So I'll just leave here the code, in case someone else would be searching for this:

import visit from 'unist-util-visit'

export async function visitAsync( tree, matcher, asyncVisitor )
{
    const matches = [];
    visit( tree, matcher, (...args) => { matches.push( args ); return tree } )

    const promises = matches.map( match => asyncVisitor( ...match ) )
    await Promise.all( promises );

    return tree;
}

Example usage:

await visitAsync( tree, 'code', visitor )

async function visitor( node, index, parent )
{
    // ! Note: Index is not guaranteed, in case of adding/removing nodes
    // ... async stuff here ...
}

Thanks for your assistance!

[wooorm]

If you’re making this into a utility, then it would make sense to correct index in there, no?

I think this is the solution to go with, no need to change this project. Thanks!

[tomasbonco]

You cannot really correct the index, can you? You would have to do it after the async stuff, but that is out of your control. Or am I missing something?

I'm not going to make it into utility, but please feel free to do it yourself.

[wooorm]

Oh, right, I was thinking this would somehow run the promises serially instead of all at once, in which case parent.indexOf(node) would do the trick.

If you can’t make index correct, I suggest not passing it, e.g.,:

visit( tree, matcher, (node, _, parent) => { matches.push([node, parent]); } )

Btw return tree in the visitor shouldn’t be there.

[tomasbonco]

You are right about index. I kept it there because in my case, I was replacing nodes, so it was convenient. However, without return tree, visit() was reporting several notes multiple times.

[wooorm]

The last part is weird! The return value of a node isn’t handled: https://github.com/syntax-tree/unist-util-visit-parents#returns 🤔 Maybe your arrow function was without braces, with push, and thus return an index (push returns a number)?

[tomasbonco]

That is exactly what happened. I wasn't aware of that behavior. Thank you!