Add list support

[mindplay-dk]

I tried to prototype an implementation of map here - and it "works", but not the way we need.

https://codesandbox.io/s/try-haptic-0-10-e9le1?file=/src/map.js

The problem here is the return newNodes statement at the end. The work is already done, so I don't need the framework to do anything here... But if I don't return anything, it wipes out the DOM - whereas, if I do return the list of nodes, it replaces the entire range of nodes, undoing the work that diff just did, making the whole thing pointless.

In my own toy Sinuous clone, this approach worked well, because reactions don't return anything that gets automatically applied to the DOM - only creation is handled at the framework level, updates are performed by the reactive listener on the DOM directly.

I'm not sure how to make this fit into Haptic.

If we have to return newNodes and let Haptic do the work, then it would need to be able to diff the result - integrating diff at that level. But that would have performance implications, I think? We would be diffing also in cases where the intent is to actually just replace, which would work, but almost definitely isn't acceptable for performance (and bundle size) reasons, right?

Any idea how this would fit with Haptic?

[tsugabloom]

so I don't need the framework to do anything here...

Perfect! Then don't use the framework 😄 Haptic shouldn't be the golden hammer for all use cases, and because it's DOM-based (a div is a div) then it's very interoperable with other libraries that do DOM work. I don't know what diff does but if it needs DOM nodes to do its job the best thing Haptic can do is get out of its way.

I forked your sandbox so map() returns a <div> then as far as Haptic can see that's it. As a framework I just won't ever touch that div again, and you can handle it in your wire. This means there's no api.patch and the wire is never mounted into the DOM so the wire doesn't need to return anything.

https://codesandbox.io/s/try-haptic-0-10-forked-nf1nb

Basically treat this problem like vanilla JS - let me know if that jives

This also means it's easier to port one of the many reconciliation algorithms (classic ivi, snabbdom, udomdiff, solid's, etc...)

[mindplay-dk]

I forked your sandbox so map() returns a <div> then as far as Haptic can see that's it.

hmmmmmm, not good enough... but I figured it out! 😄

https://codesandbox.io/s/haptic-map-2-8k0ew?file=/src/map.js

The trick was to manually run the wire, which triggers the initial render and populates oldNodes - but then, rather than returning the wire, I return the oldNodes, which is just DOM nodes, so Haptic happily mounts them, leaves them alone, and lets me handle the updates in the wire! 😀👍

I added a button to randomize the order, so you can see the reordering is working.

I did a quick test as well to see if it correctly terminates the wire when we destroy the list from a reactive parent - the "updating" messages on the console stop as expected, it appears to work perfectly. 👍

The only problem (likely unrelated to map) that I can spot, is in addFood I'm trying to clear the moreFood state using state.moreFood(""), and for some reason that doesn't happen? The value is left in the input, although it is wired with value={wire(state.moreFood)} - is that the wrong way to do it? (The same pattern appears to work fine for the checkbox state...)

My only other remark is, initially, I did this:

    {wire($ => state.showFoods() // <-- 🥴
      ? <ul>
          {map(state.foods, food => <li>{food}</li>)}
        </ul>
      : <p>I've hidden your food. 😏</p>)}

And as you can guess, this appeared to work - but it didn't update, because I forgot to pass the token.

What happened to the token being required? And explicitly having to pass that "don't track this" token instead? I mean, that seemed kind of annoying to me earlier - but apparently it may be necessary? I can see this quickly becoming a common source of bugs for people who don't test things very carefully... 🤔

Anyhow, what do you think, is this good enough to just dump into the repository for now? Then we'll have something reasonably complete and functional, and I could start porting some demos...

[tsugabloom]

The value is left in the input, although it is wired with value={wire(state.moreFood)} - is that the wrong way to do it?

No you're right and when trying your CodeSanbox I noticed that too. I fixed it in 0.10.1 https://github.com/heyheyhello/haptic/blob/main/changelog.md#0101

What happened to the token being required? And explicitly having to pass that "don't track this" token instead?

Oh that was for an entirely different usecase, one where you'd using third party libraries (which don't yet exist) who'd crash if you didn't pass a token. It's very very edge case and I just thought there needed to be a way to call functions and trick them into thinking you're passing a subtoken...

I'll think about throwing on state.count() and requiring something like state.count(R) + state.count(S) but that'd be a huge breaking change...

[tsugabloom]

Anyhow, what do you think, is this good enough to just dump into the repository for now?

I'm not sure I'd want a list reconciler in the main repo - Sinuous doesn't even have that, it's in a dedicated repo because it's a very complex algorithm. I'm also trying (maybe failing, can't tell) to avoid a monorepo structure. I'll think about it 😃

[mindplay-dk]

I'm not sure I'd want a list reconciler in the main repo - Sinuous doesn't even have that

Yes it does:

https://github.com/luwes/sinuous/tree/master/packages/sinuous/map

Most of the feature packages are available here:

https://github.com/luwes/sinuous/tree/master/packages/sinuous

it's a very complex algorithm

The diff algo is, but we didn't invent that - the rest is pretty simple.

I'm +/- on mono repos.

For something like Sinuous/Haptic that is broken down into feature packages, I think maybe it makes sense?

It might easy issue management a little? Folks will readily know where to log an issue.

Something like map does not make sense as a package by itself, since it cannot stand alone. For me, being able to deal with conditionals, lists, and component state, are basically the three pillars of any UI library. Something like map in my optic is "batteries" and should be readily available - there's no other performant way to deal with lists of things, and native map only works for things with input elements when the list doesn't change.

(This is another one of those things where I would default to using it, just for consistency, and so I don't have to refactor if I need to add an input element, or a component that features an input element, to a list of items - unless maybe we're talking about a case where file-size is critical... but that's not most projects in my world...)

The todo-list demo definitely needs this, if it's going to allow more than 10 todos 😂

[mindplay-dk]

Come to think of it, this probably isn't done.

It most likely needs to be integrated with the add/insert/remove API methods?

As of right now, it expects each item to map to a single element, so it wouldn't support fragments or "holes".

[tsugabloom]

Yes it does:

Ah yes I was thinking about luwes/memo (which isn't complicated).

I agree a list/map feature should have a home in the stdlib of Haptic I just haven't had time to focus on its design or how/why it integrates with api.* methods. I've been hacking out an redesign of how signals are created (re: #18) and trying to support both lazy and non-lazy computed signals to address the more natural support for ? : ternaries in JSX (re: #16)... It's eaten up pages of my notes trying to plan a cleaner signal design. I'll have to consider this on the backburner sorry!

I'm glad you've been able to support your map implementation outside of Haptic though :)