Skip to content
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

Problem when trying to reassign an object altered with the LS delete operator with the latter's return value #273

Closed
ymeine opened this issue Jan 29, 2013 · 4 comments
Milestone

Comments

@ymeine
Copy link

ymeine commented Jan 29, 2013

The title was hard to find for me, to make it clear and concise, but actually the problem is simple.

Whether this is a good practice or not, here is what I've done:

During one of my developments, I tried to do something I often do which is recursively traversing a tree from the root to the leaves, removing some properties on each node.
Here the tree had a simple from:

  • the root
  • each node has only ONE child node, except the leaf of course

Simple right?

Now the problem comes from the fact that the property I wanted to remove on each node during the operation was the child node itself.

So here was my first implementation:

let node = root
    while node.child?
        node = delete node.child

Considering the documentation, I would expect it to work.

However, looking at the compiled JavaScript:

(function(node){
  while (node.child != null) {
    node = node.child, delete node.child;
  }
}.call(this, root));

we see that the problem comes from the order of the operations: the node variable is assigned before the property is deleted, but here we wanted to delete a property of node.

This has been done this way to avoid a temporary variable I guess.

In the case I shew, there is a simple workaround, using an implicit variable acting as the missing temporary variable: the that reference coming from the test.

let node = root
    while node.child?
        delete node.child
        node = that

However I think this is an issue, or otherwise change the documentation to be clear on this fact.

@michaelficarra
Copy link
Contributor

Looks like a bug to me. I believe you're right about the cause, and your solution looks fine.

I wish all issues were done this well.

@vendethiel
Copy link
Contributor

Agreed with @michaelficarra.
What do we do ? Traverse LHS ?

Edit : could maybe be reporter to @satyr

@satyr
Copy link
Contributor

satyr commented Jan 29, 2013

This has been done this way to avoid a temporary variable I guess.

Yep.

Traverse LHS ?

Or just discard this optimization. It's half-baked anyway:

$ coco -bce '[a, b] = delete c.d'
var ref$, a, b;
ref$ = (ref$ = c.d, delete c.d, ref$), a = ref$[0], b = ref$[1];

satyr added a commit to satyr/coco that referenced this issue Jan 29, 2013
@vendethiel
Copy link
Contributor

satyr/coco@8410f8c (indeed) works.

tagged for 1.2

@gkz gkz closed this as completed Jul 26, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants