thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 199 TypeScript projects (and ~180 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.35k stars 149 forks source link

[rdom] $klist does not update its elements correctly? #378

Closed arnaudchenyensu closed 1 year ago

arnaudchenyensu commented 1 year ago

Hi,

I'm new to umbrella/* and I'm having some issues with rdom/$klist. Here's the example that I have:

import { defAtom } from "@thi.ng/atom";
import { div, h1, li } from "@thi.ng/hiccup-html";
import { $compile, $klist, $list } from "@thi.ng/rdom";
import { fromAtom } from "@thi.ng/rstream";
import { indexed } from "@thi.ng/transducers";

const arr = defAtom([
  { id: 1, val: 1 },
  { id: 2, val: 2 },
  { id: 3, val: 3 },
  { id: 4, val: 4 },
])

$compile(div({},
  h1({}, '$klist:'),
  $klist(
    fromAtom(arr).map((arr) => [...indexed(arr)]),
    "ul",
    {},
    ([index, { id, val }]) => li(
      {
        onclick: () => arr.resetIn([index, "val"], val * 2),
        style: { cursor: "pointer" }
      },
      `${id}: ${val}`
    ),
    ([_, { id }]) => id
  ),
  h1({}, '$list:'),
  $list(
    fromAtom(arr).map((arr) => [...indexed(arr)]),
    "ul",
    {},
    ([index, { id, val }]) => li(
      {
        onclick: () => arr.resetIn([index, "val"], val * 2),
        style: { cursor: "pointer" }
      },
      `${id}: ${val}`
    ),
  )
)).mount(document.body);

I'm expecting the values to be correctly multiplied by 2 but it's only working for $list, am I doing something wrong?

Btw, I'm really loving your work! I've been watching it for a while now but it felt overwhelming at the time...

Thank you for your time, Arnaud.

postspectacular commented 1 year ago

Hi @arnaudchenyensu - thank you so much for getting in touch about this. I know some of the packages can be a bit challenging to get started, but this kind of feedback is very helpful to also learn about what newcomers are struggling with...

In this case, the solution is actually very easy and before I explain it, I also want to point you to this part of the $klist docs:

https://github.com/thi-ng/umbrella/blob/c0731fab1ab9fd30181f97b1da82858df99c5384/packages/rdom/src/klist.ts#L21-L22

So in other words, a list item's unique key value MUST be not just unique in terms of structure (e.g. like your ID attrib), but also take into account something else, e.g. the item's current value. This is because internally $klist only identifies list item changes via changes to these composite keys and nothing else. So with that said, a possible easy solution here would be using this as key function for your $klist:

([_, { id, val }]) => `id${id}:${val}}`

Altogether:

$klist(
    fromAtom(arr).map((arr) => [...indexed(arr)]),
    "ul",
    {},
    ([index, { id, val }]) =>
        li(
            {
                onclick: () => arr.resetIn([index, "val"], val * 2),
                style: { cursor: "pointer" },
            },
            `${id}: ${val}`
        ),
    ([_, { id, val }]) => `id${id}:${val}}`
),

Hope this all makes sense. I'll update the comment & mini example in the klist docstring to make this behavior more obvious... If you've got any other ideas, please do let me know! 👍

(Ps. also thank you for the kind words!)

arnaudchenyensu commented 1 year ago

Hi @postspectacular, thank you for your answer, it's a lot easier to understand. To be honest, even by reading the docs of $klist I find it hard to understand that the function was expecting something else than just a "normal" id. If you rephrase it just like your answer, it would be perfect :)

As a personal feedback, because I'm coming from a React/Vue/etc. background, I was expecting all the values to be tracked/reactive by default. For example, I was expecting the style attribute to change automatically (took me a while to understand that I needed to use fromAtom(val).map):

style: { "background-color": val.deref() === 1 ? 'blue' : 'white' }

I would also recommend putting an example with $list/$klist on rdom's readme. I thought that as long as I start the root of my tree with a stream (e.g div({}, fromAtom(), ...)) my list elements would update automatically.

Again, it's just a personal/first impression/on boarding feedback, not a criticism :)

Keep up the good work!

postspectacular commented 1 year ago

Understood & thank you for the feedback! I've updated both the docs and the readme, hopefully making the behaviors a little more clear! :)

As for the auto-tracking & 'everything reactive by default' - that's exactly why all of these other tools require their enterprise-scale elaborate/complex pre-processing/transpilation tooling, something I've been strictly & fundamentally avoiding with all projects in this repo. In thi.ng/rdom, there's no "magic" reactivity or any form of centralized/auto-managed behind-the-scenes coordination and all reactivity and other control flow is forced to be explicit (by design). This not only provides super fine-grained control (see thi.ng/rstream & thi.ng/transducers for various reactivity/transformation building blocks), but also allows rdom to be absolutely nimble, components to be super simple, mix & match declarative/imperative updates, perform minimal updates without any need for diffing, and IMHO is just easier to reason about. I'm well aware this approach is not popular in the wider JS/frontend community, but in other language camps (e.g. #Clojure, now also #Zig) this 'no magic' and 'no hidden control flow' attitude is seen as one of the main design principles & net positives, even if it makes some parts more verbose (one can always build some cheap abstractions around that, if needed)...

As a more general comment about the design of umbrella packages: Most of the provided packages here started out as relatively low-level building blocks and basis for (slowly) building up higher level tooling/convenience layers, to experiment with different techniques & approaches. For years, I've been doing that myself, but also was always kinda hoping/relying that other people would do so too (and then maybe even share/feedback some of those results - not so much, yet...). This design approach & focus on composability is totally counter to the much more popular all 'batteries-included' or 'zero-conf' myths and their constant re-invention of the wheel... To me composability and reasoning is 1000% more important!