Make VFileContents generic to support processors that return objects

[sandhose]

Subject of the feature

Right now, in the type definitions, VFileContents is either a Buffer or a string. In some unified processors, the VFile content is an object, which makes them impossible to typedef them correctly.

Problem

For example, remark-react returns a ReactNode in its compiler, making it impossible to define types for it. Please note that patches would also be necessary in unified types to support processors that modify the VFile content type.

remark-react is not the only compiler concerned by this. I'll open issues in the projects I mentioned here related to this.

Expected behaviour

VFile should be a generic for its content. The default can be Buffer | string in order to be fully backward-compatible. With this done, we should be able to declare the remark-react compiler output as something like VFile<Contents = ReactNode>.

Alternatives

An alternative would be to declare the VFileContents as any. It would be a lot easier to do, but we would loose a lot of strict type checking.

[wooorm]

We had a conversation about this here too: unifiedjs/unified#47 (comment)

/cc @Rokt33r @ChristianMurphy

[sandhose]

Well, casting works as a workaround ; I'll try to do a PR that adds the type parameter on VFile without breaking existing type definitions in other libraries.

From what I see in type definitions, the hardest part will be on the unified side because there are no real distinction between Compilers and other types of processors.

[ChristianMurphy]

I'd be okay with exploring what generalizing vfile contents may look like.
Without seeing a PR, not sure how far reaching the implications would be.

Getting unified to correctly inference the type of content could be problematic (see: unifiedjs/unified#84 (comment))

[Rokt33r]

I disagree about introducing generics. But the alternative sounds good to me. If remark-react were outside of remarkjs, I would say the customized VFile interface(having other content type than string or buffer) should be defined in remark-react though. Now it's in remarkjs now. So it might be nice time to update our VFile interface spec.

[wooorm]

remark-react / remark-vdom / rehype-react don’t have types yet. Would it be possible to define types that solve this there, compared to changing vfile?
VFiles should have string or buffer contents, returning objects is a bit hacky, IMO not preferred, but it’s the best way I can think of to deal with this.

We could start using a different property for virtual DOMs as well, e.g., vfile.vdom?

[Rokt33r]

remark-react / remark-vdom / rehype-react don’t have types yet. Would it be possible to define types that solve this there, compared to changing vfile?

It is just putting away the problem to other place. If we do so, unified would need a generic parameter to determine return type of process and processSync.

So, in my opinion, the easiest option should be allowing any type for contents of VFile interface but not for parameter of vfile function for now.

[wooorm]

process and processSync already return a vfile, that doesn’t need to change. What I mean with having some compilers set a file.vdom or so, would also mean that file.contents is not modified.

A compiler could then look like so:

function rehypeReact() {
  return serialize
}
function serialize(tree, file) {
  // Set result somewhere different
  file.vdom = toReact(tree)
  // Return the current value to not change anything
  return file.contents
}

[Rokt33r]

What I mean with having some compilers set a file.vdom or so, would also mean that file.contents is not modified.

It sounds great! I think we should do like that although it causes breaking changes but much more makes sense.

[wooorm]

Should be start doing one 'vdom' property? If so, other naming suggestions? If not, I guess we go with namespaced keys directly on file or on file.data, e.g., like file.data.reactTree?

[Rokt33r]

file.data.reactTree sounds better to me.

[wooorm]

Alternatively, this could be solved by unified: unified has a “compiler” concept, and compiling is a broader term that serializing. Serializing is also compiling, but to text.

What if unified takes the result of that compiler (which could be the compilers from remark-stringify or remark-react), checks if the result is string, Buffer, or nully, and based on that either sets vfile.contents (vfile.value, see GH-49, GH-50) or vfile.result?

I think I like this better because plugins won’t need to update.

[Rokt33r]

@wooorm Sounds good to me. Serializing feels too limited to me.

[ChristianMurphy]

checks if the result is string, Buffer, or nully, and based on that either sets vfile.contents or vfile.result

This may work.
Thinking of edge cases

1. what if the return type is a class instance which implements a custom toString?

class Example {
  toString() {
    return "<h1>hello world</h1>";
  }
}

// where the compiler returns
new Example();

// which when used as a string
`${new Example()}`
// logs "<h1>hello world</h1>"

Is this something that would be considered a value or a result?

2. Depending on how you interpret "nully", would that include falsy values? Like number with 0, boolean with false, would those be split? (E.G. 0 would go to vfile.contents, 1 to vfile.result)