feat: use xdmEsBuild for all compiling

[Arcath]

Big thanks to @gaelhameon for the issue #41 and the code in there that helped me with esbuild! I've added you as a co-author as your fix in #41 was needed here.

Let me know what you think of the code in this PR.

What this does it removes the xdm parsing from inMemoryPlugin and puts it into its own plugin. This new plugin can read mdx from the disk and node_moudles fixing #41 and means that there is no need for users to configure xdm for disk files as its all going through one parser now.

• Documentation
• Tests
• Ready to be merged
  

[Arcath]

I should explain a bit more about why we need our own plugin.

The in-build xdm plugin starts its onLoad function with a call to fs.readFile (here) which means we can't push our inMemory content to it. I don't think we can ask @wooorm to put in a special case for just for mdx-bundler, unless there is a consistent way to do it in esbuild.

The only way I can see esbuild doing that is instead of plugins using fs.readFile they use build.getFile and essentially make our inMemory plugin a core feature for injecting strings as files. Again it feels a bit too niche an issue for another project to do just to improve mdx-bundler.

[wooorm]

I’m not much of an esbuild export, but I imagine that plugin chaining isn’t that weird of an ask.

I was under the impression that esbuild plugins only got file paths.
Can your InMemoryPlugin pass contents to xdm?
It doesn’t seem it can, according to the docs?

[gaelhameon]

Hi @Arcath

Thanks for this !

I also made a PR #46 where I simply replace return {} by return undefined and it solved the issues I had. Are there other cases that can't be solved like that ?

Nevertheless, extracting all the mdx compiling/bundling and putting it in the same place no matter where the data comes from seems like a good idea. On the other hand, using our own plugin means we don't benefit from all the options of the generic xdm plugin. For example, built-in management of multiple extensions: #45 (comment)

Maybe we could do this but also leave in the original xdm plugin as a last resort ?

[Arcath]

@wooorm I'm passing it here through the pluginData field in esbuild so inMemory can put the contents in there and then read it in xdmPlugin. I suppose a proper chaining API would make sense.

@gaelhameon thanks! I've updated the filter matcher. I went down this route along with your solution as it seemed odd to me that we would be asking users to compile some mdx through our compiler and then other mdx through a different one.

We could re-add the xdmESBuild plugin, although since it would most likely need re-configuring when used is it better to document how to do it and let users throw it in if the need it? ping @kentcdodds

[wooorm]

I’m not averse to accepting a contents or so (maybe value?).
Let me check through the existing esbuild plugins list to see if some other name is in use already

[Arcath]

I’m not averse to accepting a contents or so (maybe value?).
Let me check through the existing esbuild plugins list to see if some other name is in use already

We could use a pluginData of {xdm: {contents}} leaving room for expansion and hopefully no collisions with other plugins?

[wooorm]

I checked a couple, and see barely any use of pluginData in any of the existing plugins. Only when one user-facing plugin is essentially two plugins.

I think pluginData.contents is good. I’m not afraid of collisions, as a) almost nobody is using pluginData, b) contents is also often returned from plugins.
Using this to “pass through” and chain plugins seems good to me!

[wooorm]

Or should xdm also support “virtual” modules (https://esbuild.github.io/plugins/#namespaces)? 🤔

(edit: or should that be done with the in memory plugin here?)

[Arcath]

Or should xdm also support “virtual” modules (https://esbuild.github.io/plugins/#namespaces)? 🤔

(edit: or should that be done with the in memory plugin here?)

I'll have a play with namespaces, the 2 plugins in this PR are probabbly a good testbed for that.

[kentcdodds]

I see this change as a definite improvement 👍

I haven't read through the conversation yet, but from what I see this is a ✅ from me :)

[kentcdodds]

Ok, so the thing I like about this change is it means that the configuration people set via xdmOptions will apply to all MDX files, not just the ones in memory. If we can have that and use the official xdm plugin then that's even better :)

[Arcath]

@kentcdodds my thoughts exactly, the easier we can make it the better.

I've been working with namespaces and getting a little stuck with them. It appears that once you have said a file belongs to a namespace all the files it imports also belong to that namespace.

This is leaving me a little stuck when it comes to files that esbuild should handle. By the time inMemory is saying "use the file loader" its too late and it wont try.

I don't think namespaces work quite how we need them to.

If you look at the sample http plugin it see the http:// in an import and sets the namespace so that esbuild and any other plugins wont try to resolve the imports as import sample from './sample' needs to be run in the http plugin only, it wont be interacting with the filesystem ever again. We need to jump in and out of virtual and real files, as well as in and out of mdx, js, jsx. Hence messy.

Thats leaving me thinking short term @wooorm could add support for reading from pluginData.contents if it exists and a solution could exist in esbuild in the future. I've opened an issue on esbuild to talk about it evanw/esbuild#1243

[Arcath]

@wooorm Would it be possible to go with a temporary work around whereby the XDM plugin will accept contents in the pluginData?

For now esbuild doesn't support this and it is a bit of a shift in the API that imagine wont be coming for a while.

[wooorm]

Yeah, sure! Can you work on it?

[kentcdodds]

This looks good. Breaking change, but that's fine. And I'm happier with this :) Just a few smaller things.

[Arcath]

Thanks @wooorm for merging that PR :) This now use your esbuild plugin exclusively!

@kentcdodds This is now a breaking change as it removes the input parameter of xdmOptions as it isn't available anymore.

[kentcdodds]

LGTM 👍

[github-actions]

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀