Add a TodoMVC example - Githubissues

wizzard0 commented 7 years ago

While it might feel dated, it actually illustrates the basic interplay between a lot of moving parts (persistence, aggregate values, view filter, etc), and is simple enough to fit in one file.

devsnek commented 7 years ago

i just started working on this 😄

devsnek commented 7 years ago

@gliechtenstein should modifying cell.$components actually update the components? I'm not seeing that happen, and it is making rendering this list very difficult.

gliechtenstein commented 7 years ago

@devsnek yes it's supposed to update $components.

But there's a gotcha (maybe you're running into this) https://github.com/intercellular/cell/issues/3

Let me know the solution in that link solves the problem. Otherwise please share a minimal piece of code that can be replicated and I'll take a look. Thanks!

devsnek commented 7 years ago

@gliechtenstein right now i have

  _add(item) {
    this._items.push(item);
    this.$update(); // this call
    return true;
  },
  _delete(item) {
    const index = this._items.indexOf(item);
    if (index !== -1) {
      this._items.splice(index, 1);
      this.$update(); // also this one
      return true;
    } else {
      return false;
    }
  },
  $init() {
    this.$update();
  },
  $update() {
    this.$components = this._items.map(i => todoItem(i));
  },

$update is called only if i have those this.$update calls, and modifying $components does nothing

also it looks like things like this.checked when making a checkbox isn't properly binding to the checked property on the element

gliechtenstein commented 7 years ago

@devsnek only the attributes "declared" on the gene will auto-trigger $update(). This means you need to have an _items key defined somewhere on the gene object. Otherwise it won't trigger. More on this over at https://github.com/intercellular/tutorial#important

Also here's a fiddle that may help: https://jsfiddle.net/zk6s1c3z/

Let me know if this fixes your problem!

piranna commented 7 years ago

I know Object.observe() was removed from spec and Chrome although there are some polyfills, but I think there was a DomObserver that's widely implemented and can be used to monitor the newly added attributes...

El 30/06/2017 22:47, "gliechtenstein" notifications@github.com escribió:

@devsnek https://github.com/devsnek only the attributes "declared" on the gene will auto-trigger $update(). This means you need to have an _items key defined somewhere on the gene object. Otherwise it won't trigger. More on this over at https://github.com/intercellular/tutorial# important

Also here's a fiddle that may help: https://jsfiddle.net/zk6s1c3z/

Let me know if this fixes your problem!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/intercellular/cell/issues/28#issuecomment-312369368, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgfvps3cq-FGL2Sh7yL4QI2H58g8E5Pks5sJV7hgaJpZM4N9kKZ .

gliechtenstein commented 7 years ago

@piranna yup we're using something similar to Object.observe(). We use Object.defineProperty. Just to clarify, Cell doesn't update the DOM directly and that's the whole point of the library--it lets you store variables on the DOM without any side-effects such as reflows on attribute changes--I think It would be extremely inefficient if we attached all the attributes directly on each element AND monitored them via DOM observer. Plus, I don't think it's even possible to monitor full fledged objects using DOM observer, with DOM observer you can at best track attributes which are strings.

Cell maintains a proxy object using Object.defineProperty which is much more efficient and portable. We could have used ES6 proxy, but it's not supported in all browsers (not even IE11). We may transition to ES6 proxy someday in the future when all browsers support proxies (In fact, the first version of Cell used proxies, and I had to come up with a clever way to get rid of it. Also the proxy polyfills are not really polyfills because underneath they actually use Object.defineProperty). Anyway, I think we should stay away from using the web API but use pure Javascript methods as much as possible because web APIs depend on browsers.

Also the explicit definition of variables is a feature not a bug, it's like how you declare public variables when defining classes to let the outside world know how to interact with the class. Hope this makes sense!

devsnek commented 7 years ago

@gliechtenstein i do have an _items array on the gene, sorry for leaving out of that snippet.

gliechtenstein commented 7 years ago

@devsnek can you share a fiddle? Not the whole thing, but if you could isolate the problem in as few lines of code as possible..

devsnek commented 7 years ago

@gliechtenstein https://github.com/devsnek/todomvc/blob/master/examples/cell/app.js

gliechtenstein commented 7 years ago

@devsnek this is because you're trying to call these functions on regular objects. These gene objects are nothing more than a regular javascript object. Even after they get instantiated into a Cell element, nothing changes to window.todoList, it's exactly as you have written.

This means when you call window.todoList._delete(item);, the this context inside _delete will be the window object. You should be calling the _delete function on the generated element instead.

So, what would work is if you call

document.querySelector("#todo-list")._delete(item);

instead of this line

But this brings up an interesting issue, I see what you were trying to do there, maybe we could come up with simpler ways of referencing generated cells instead of doing document.querySelector all the time.

Lastly, I didn't understand your comment about the checked attribute. If above solution doesn't fix the checked issue, please could you elaborate?

devsnek commented 7 years ago

also setting style on a cell breaks HTMLElement#style, overriding it with the string.

ghost commented 7 years ago

But this brings up an interesting issue, I see what you were trying to do there, maybe we could come up with simpler ways of referencing generated cells instead of doing document.querySelector all the time.

@gliechtenstein I was thinking about this over the last few days and while I don't have a fully formed thought but I do have some observations.

First is that there appears to be three options for referencing child elements.

this.querySelector:
- Pros:
- traverse down the DOM skipping elements which serve no special purpose but for markup.
- Can be more concise.
- Cons:
- using strings as query lookups, easy for typos and not good for refactoring.
- requires now handling null results if elements not found.
- What if I remove the class or id, I've now broken application logic.
- can't query on cell attributes, only on the class/id.
- Also classes in my mind should be style only *imho. This could be mitigated by constants but ¯_(ツ)_/¯.
walking $node.children
- Pros:
- Explicit, with little magic.
- View element's properties and match any value. "I want all child nodes with _completed: false and _hamsters.length > 15". More complex pattern matching. Oh Elixir how you'd do well here...
- Cons:
- Explicit but verbose if walking past markup elements. See above.
- Require a priori knowledge of DOM structure.
Custom walk/query. Function that walks children and matches list of predicates. Maybe with depth limits.
- Pros:
- Skip markup nodes
- Expressive queries. "I want all child nodes with _completed: false and _hamsters.length > 15"
- Cons:
- Expanding the cell.js codebase.
Final option (most abstract), nodes with $cell: true or with some other identifying flag are bound to variables in the most near parent element with $cell: true so like $cell.cellChilderen. In my mind, there are stateful cells and view cells. If you have reference to these child cells you can send messages to them and vise versa. I'm thinking in terms of actor model supervision trees like in Erlang/Elixir.
- Pros:
- Direct linking of nodes meaning you know if they exist or not instead of handling null queries.
- More explicit access.
- Cons:
- More complex inner workings of the library.
- Could be secondary library on top of cell.js?

ghost commented 7 years ago

Wanted to add another option which in some ways is better because of an inversion of control. That is using something like Redux or insert n variations of Redux.

Some of the key benefits are; having a single source of truth, subscriptions are context local to state tracking cells and there's no need to have an identification schema because it's pulling data rather than pushing. Now of course local to the component data can be pushed down or whatever there's lots of options once you get the main chunk of state you're subscribed to. This of course has the limitation listed in #118 where you need to unmount the subscription.

gliechtenstein commented 7 years ago

@kingoftheknoll wow thanks for the thorough list of options. I like the "Final option" on your first comment. I also had some ideas along the same line of thought but I haven't been able to yet think of an approach I really like, but I have a feeling we're on the right track.

I think some variation of this approach would be best since we can implement the reference while preserving the decentralized nature. Let me think about this some more and update the thread as I come up with some ideas even if they're rough. Please don't hesitate to share more ideas on this when you come up with them too.

BTW just to clarify, the $cell: true currently signifies that the object that directly contains that attribute needs to be inserted into the DOM tree, so having $cell: true somewhere down the tree should not happen. (All global objects with $cell: true are plugged in as a root node.)

As for the redux idea, I think this can be something that can be built on top of Cell but doesn't necessarily have to be a built-in feature. In fact if we make this the default, it will break the decentralized nature of Cells because redux is a very centralized approach. I'm not saying that centralized approach is bad, and actually I think it's the easiest way to implement apps like todo list because these apps by nature are centralized (the TODO database is the single source of truth), but even then I think it would be ideal if the centralized approach can be built on top of a perfect decentralized solution. Hope this makes sense :)

gliechtenstein commented 7 years ago

@devsnek I just pushed an update that fixes the style problem https://github.com/intercellular/cell/pull/138 (Related thread https://github.com/intercellular/cell/issues/136) Hope this now fixes all the problems!

ghost commented 7 years ago

@gliechtenstein I agree that Redux like state management should be optional. I'm more saying it might be the most ergonomic thing because of the inversion of control.

On the flip side, if one is to push data down the question because where does the responsibility of the mutations lie? Cell.js already has a split view on this issue. First choice using the DOM api to select and mutate elements. Second is to delegate to the $components getter/setter which from an api standpoint is not too dissimilar to the declarative nature of React. Basically "here's what I want it to look like, draw it". But that's rather heavy handed and in my mind not the best choice for say, updating the background color of an element. Rather maybe it's a good choice for redrawing a table when paginating.

Which leads me to the point I keep coming back to which is there needs to be a conceptual/api boundary, dare I say a component with an api wrapping both choices. Now as a FP guy I really like delegation to something to do the side effects but since we want to sit on the DOM (dumpster fire of mutable state) I think an actor model is probably the best interface. I don't think you should ever from the root of the DOM tree query down and start mutating stuff willy-nilly. Rather, I think messages should be sent to the inboxes of components where their can do mutations or delegate messages/mutations to their child components. Coming from Elixir, I think about this in terms of it's OTP process supervision trees.

So from that world processes get a PID. You can explicitly send to PID's but most often you use a PID registry. So I can imagine when starting with the JSON api, you can wrap a number of JSON dom elements with a string or tuple unique identifier and a handle_call function that will match on message key. When that root element of the component is created, a PID is created, register to Registry and set the handle_call function on that dom element. So then from the parent nodes you can send messages through the registry to the components using whatever pattern you used to register them.

ghost commented 7 years ago

Slight correction to what I said before. You would put the mailbox on the Genotype not the DOM element. And registering an identifier would be pointing back at the Genotype's inbox.

This also tickles my memory about how React and friends efficiently do updates on lists which is to give a unique id to each element in the list for fast targeted updates. That's not too different than what I'm proposing.

Another way to think about a registry would be to not have a global registry but rather a registry local to your immediate parent registry. But that would mean you can only send messages up and down the dom tree not across it... maybe not a good idea then =)

gliechtenstein commented 7 years ago

@kingoftheknoll just realized we're on a TODOMVC thread haha, I just created a separate ticket so we can leave this ticket alone. If anyone else is interested in this "centralized structure" discussion as well, please check out: https://github.com/intercellular/cell/issues/143

SilverSoldier commented 6 years ago

Created a basic TodoMVC. Link to repo.

intercellular / cell

Add a TodoMVC example #28