If-expression inside tags gets wrong cache key about imba HOT 13 CLOSED

We need to rework the whole tag-caching to cache alle descendants regardless of depth on the outermost tag (in this case ). By the way, render is currently not called by default now (need to call render in build-method on tags). Should probably make it the default. I'll look into this first though.

from imba.

It would also be nice to be able to cache based on a key, but not use an instance variable:

<self>
  for data in posts:
    <post id=data.id title=data.title>

If the content of posts changes order, it would be preferable to re-use the same DOM-node. React solves this by having a magic property (key) which is being used for caching.

Another issue: When/how to delete old nodes from the cache.

There's probably no point in creating many issues though. It will all be solved by the same solution, I guess?

from imba.

Good idea. Personally I've always created special collection-nodes that deal with caching and rendering of collections, so I rarely use for-loops for this. That said, it should be easy to use a special key-property. Then it wont be possible to actually use the key property for anything else though. Would you prefer <post key=task.cachekey> or a custom syntax like <post@[task.cachekey]>? I guess key= looks cleaner, and it's probably smart to follow react conventions when possible.

from imba.

Fixed the reported issue: 3f64d6c?diff=unified#diff-a2e3ab5a861205010ad451ad9ba826e0

from imba.

This depends on what conventions you'd want in Imba. I like having for … in … as a general performant way to render collections. I guess it also depends on how you want to solve animations.

It feels a bit awkward to special-case the key property (since it would be handled at compile-time), but it might be better than introducing another syntax.

from imba.

{ } is already used for interpolation many places, like if you want to add dynamic classes:
<post .{category}>, so maybe <post@{key}> would make sense. Another question is whether the tags inside a for-loop with cachekeys should be cached on the outermost tag of the tree, or the parent.

from imba.

Cached on the parent has a clear advantage when you want multiple lists of the same data:

<h1> "My Page"

for section in sections
  <li@{section.id}>
    <a> section.title

for section in sections
  <div@{section.id}>
    <h2> section.title
   <div> section.content

This could compile to:

(this[1] || (this[1] = t$("div")).setChildren([
  (this[2] || (this[2] = t$("h1")).setContent("My Page"),
  (function(scope) {
    var arr = [];
    for (…) {
      (scope[section.id] || (scope[section.id] = …))
    }
    return arr;
  })(this[3] || (this[3] = {})),
  …
]);

from imba.

Oh absolutely, that's true. The only issue then is the fact that tags with an explicit ivar/cachekey is cached on the outermost scope, and with the <post@{key}> syntax one would intuitively think that it works the same way as <post@key>. Maybe we should make regular <post@key> cache on the parent as well (instead of on root) but I'm not sure.

from imba.

One concern with conditionals/loops and keys:

if important
  <foo@header type="important"> "Hello"
else
  <foo@header> "Hello"

If this is being rendered first with important = true, and then with important = false, there's nothing which sets type=null.

from imba.

<post@key> should probably be disallowed in loops. I don't see any way for it to make any sense inside a loop. One solution is to make <post@key> and <post@{key}> more distinct: We could make <post@{key}> is only for caching and not for access (e.g. it sets an ivar named "_hidden_"+key or something). That way, you don't expect <post@{key}> inside a loop to be accessible.

from imba.

I think these rules are sensible for Imba:

• <tag@key> must be unique. My example with conditions should not compile. Using <tag@key> inside loops are disallowed as well.

• <tag@{key}> allows caching, but not access. It's cached on the root, but with a cache key that's a combination of the static ID and the key. Thus, my example of iterating over the same list multiple time works because the static ID is different.

  for foo in bar
    if foo.something
      <foo bar=1>
    else
      <foo>

  In this case the two tags have different cache keys.

  If you want them to be cached together you need to write:

  <foo bar=foo.something and 1>

• <tag> (without {key}) inside loops is never cached.

With these rules you know that Imba will only re-use/cache a DOM nodes from a tag on the exact same position in the source. This seems quite sensible to me.

from imba.

I agree about the dynamic keys. I'm not sure the compiler should require regular keys to be unique though. render is not a special method, and you will always be able to use the same key other places.

tag post
    def render
        <self> <div@header> "I am header"

    def mess
        <self> <div@header> "Am I the same?"

Since this is possible, we should possibly embrace it instead. As long as you understand what is actually going on, the example you presented still makes sense:

if important
    <foo@header type="important"> "Hello"
else
    <foo@header> "Hello"

Sure, if important == true during one render(), type will be set to "important". If important != true, type will not be touched at all. It is only confusing if one expects a full rebuild (like React?). In fact, this behaviour could actually have some interesting usecases, ie if you want the render method to mark the node / add something if some condition has been present at some point in time.
If I really want the other condition to 'reset' everything, I could write:

if important
    <foo@header type="important"> "Hello"
else
    <foo@header type=null> "Hello"

A basic implementation of @{} has been pushed. You can see a simple example at http://imba.io/#tags/caching.imba

from imba.

By the way @judofyr, the current implementation of cachekey caches nodes on their parent, not on root. I figured that might be better for cleaning up / pruning inactive elements. If all references to the parent are removed, all references to the inner cached elements will automatically be removed as well. And it would be easy to have a prune-method that simply loops through cached elements on a node, and delete all cached elements that are currently not in the dom-tree of node.

from imba.