New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should we add a updateSelf() API? #1748

Closed
xieyu33333 opened this Issue Apr 16, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@xieyu33333
Contributor

xieyu33333 commented Apr 16, 2016

if My tag have some complex hidden custom tag, for example:

<wrap-tag>
    <div> .........//here is my tpl </div>
    <tree id="tree-1"></tree>
    <tree id="tree-2" show="{ false }"></tree>
</wrap-tag>

if I update the wrap-tag, I will also update the trees, if the trees have too many items, the page will be very slow.

the same example

<wrap-tag>
    <div onclick="{ switch }"> .........//here is my tpl </div>
    <tree id="tree-1" show = { !boolean }></tree>
    <tree id="tree-2" show="{ boolean }"></tree>

   switch(e) {
       this.boolean = !this.boolean
   }
</wrap-tag>

I only want to swich the display of the trees , but the trees will be update, I think riot should use updateSelf() instead of update(); if I need update(); I can use it by myself.

so should we add a updateSelf() API? only update wrap-tag itself.

@rogueg

This comment has been minimized.

Show comment
Hide comment
@rogueg

rogueg Apr 17, 2016

Member

We've been talking about adding a shouldUpdate API, which would let you skip updates to a tag when you know they're a no-op.

So inside of the tree implementation, you could add:

shouldUpdate() { return false }

And then the tree would never update. Of course you could add logic there to check if some global state has changed as well. Do you think that would solve the issue?

Member

rogueg commented Apr 17, 2016

We've been talking about adding a shouldUpdate API, which would let you skip updates to a tag when you know they're a no-op.

So inside of the tree implementation, you could add:

shouldUpdate() { return false }

And then the tree would never update. Of course you could add logic there to check if some global state has changed as well. Do you think that would solve the issue?

@xieyu33333

This comment has been minimized.

Show comment
Hide comment
@xieyu33333

xieyu33333 Apr 18, 2016

Contributor

@rogueg I think that would not solve the issue, for example:

  <wrap-tag>
    <div> .........//here is my tpl </div>
    <tree id="tree-1" opts="{ treeData1 }"></tree>
    <tree id="tree-2" show="{ false }" opts="{ treeData2 }"></tree>
</wrap-tag>

if treeData has changed, I must update the tree, but in other actions, tree should not be update.

Contributor

xieyu33333 commented Apr 18, 2016

@rogueg I think that would not solve the issue, for example:

  <wrap-tag>
    <div> .........//here is my tpl </div>
    <tree id="tree-1" opts="{ treeData1 }"></tree>
    <tree id="tree-2" show="{ false }" opts="{ treeData2 }"></tree>
</wrap-tag>

if treeData has changed, I must update the tree, but in other actions, tree should not be update.

@rogueg

This comment has been minimized.

Show comment
Hide comment
@rogueg

rogueg Apr 18, 2016

Member

@xieyu33333: It depends on how your data changes. You'd write something like this in tree

shouldUpdate() {
  if (opts.opts == this.oldOpts) return false
  this.oldOpts = opts.opts
  return true
}

That would would work if the entire treeData object changes, but not if only one part changes.

Member

rogueg commented Apr 18, 2016

@xieyu33333: It depends on how your data changes. You'd write something like this in tree

shouldUpdate() {
  if (opts.opts == this.oldOpts) return false
  this.oldOpts = opts.opts
  return true
}

That would would work if the entire treeData object changes, but not if only one part changes.

@juodumas

This comment has been minimized.

Show comment
Hide comment
@juodumas

juodumas Oct 28, 2016

Contributor

Looking at the implementation, when shouldUpdate returns false, this.update(data) won't set data on this. This is unexpected and creates a problem:

this.updating = false
this.shouldUpdate = () => this.updating
// this.onclick() won't set updating to true because shouldUpdate() will prevent it.
this.onclick = () => this.update({updating: true})
Contributor

juodumas commented Oct 28, 2016

Looking at the implementation, when shouldUpdate returns false, this.update(data) won't set data on this. This is unexpected and creates a problem:

this.updating = false
this.shouldUpdate = () => this.updating
// this.onclick() won't set updating to true because shouldUpdate() will prevent it.
this.onclick = () => this.update({updating: true})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment