tbranyen/diffhtml

Add `createNode` to Transaction#internals

mAAdhaTTah opened this issue · 10 comments

For my purposes, I needed to reimplement the patchNode function, and the Transaction object currently provides all of the internals needed to do so, rather than importing them from diffHTML from a non-public path. createNode is the only internal function that isn't accessible this way, so I'm currently pulling it in from diffhtml/lib/node, which I'd rather avoid. Can that be provided on the Transaction object?

@mAAdhaTTah I agree that digging into the module structure is probably not ideal. What we could think about doing is deprecating the internals concept in core and instead maintain a package that digs into the module structure and exposes the internals safely since we can maintain compatibility safely with the wrapper.

Could look like:

exports.createTree = require('diffhtml/lib/tree/create');
exports.makeTree = require('diffhtml/lib/tree/make');

/* etc... */

Thoughts?

I'm okay with adding createNode to the internals object, but I don't think it should be directly attached to the transaction object.

There is an internals object already attached to the Transaction object, which is why I thought that was an acceptable location. Whatever mechanism you think is most maintainable from your end works for me; just looking for a "blessed" way of accessing some of those internals, especially because once the module is published, you can easily run into weird bugs if you import the main entry from the dist folder and some internals through lib and end up references different instances of the various caching mechanisms.

@mAAdhaTTah we're talking about the same thing when we say internals object. I'm okay with putting it there. Feel free to PR that, I might have time during lunch today to do it otherwise.

Ah ok. Looking at the code now, it looks like the internals object is just a namespace import of the util folder. Should we re-export from that module's index.js? And how should it be re-exported? export * as Node from '../node'; or each function individually? And should we re-export the Tree namespace in this way?

And not to go too far down the rabbit hole, but feasibly, the built-in tasks could use the internals object themselves so they could work as examples for anyone looking to implement their own tasks.

I'd like to mess with the idea of a separate internals package before we jump too fair down the rabbit hole. Ideally the internal tasks will use the same exact API as a consumer would to dogfood the concept :-p

Ok, I'm happy to handle a PR if you set the direction. Let me know what you'd prefer.

I'm experimenting with a diffhtml-shared-internals module that exposes the internals from diffHTML in a safe way. The reason why I'm moving slow on this, is that I'd like to try and suss out enough of the API to avoid needing any kind of rewrites later on. I don't have it figured out yet, it's going to take time. I'm focusing on the React Like Component implementation and I want to have that polished with all lifecycle methods bound with the most minimal integration necessary. Right now some use cases are a bit hairy and need more thought.

I'll open up a PR soon so you can see what I have. Luckily this first implementation won't really require much changes on your end other than an imports change.

Cool looking forward to it.

Resolved by #129.