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

setting individual array elements not working #1854

Closed
pvmart opened this issue Jun 12, 2015 · 15 comments
Closed

setting individual array elements not working #1854

pvmart opened this issue Jun 12, 2015 · 15 comments
Assignees

Comments

@pvmart
Copy link

pvmart commented Jun 12, 2015

Binding specified by path eq [[test.0.value]] or dom-repeat have nothing in common with actual data in model.
Given:

         properties: {

           test: {
             type: Array,
             value: [{value: 0}],
             notify: true
           }
         },

         ready: function() {
           this.unshift('test', {value: 1});
           this.set('test.0.value',     2);
           this.set('test.0',   {value: 3});
           this.set('test.0.value',     4);
           this.push('test',    {value: 5});
           this.$.model.innerText = JSON.stringify(this.get('test'));
         }
        <header>static path [[test.0.value]]</header>
        <div>[0] <span>[[ test.0.value ]]</span></div>
        <div>[1] <span>[[ test.1.value ]]</span></div>
        <div>[2] <span>[[ test.2.value ]]</span></div>

        <header>dom-repeat</header>
        <template is="dom-repeat" items="[[test]]">
          <div>
            [<span>[[index]]</span>] = <span>[[item.value]]</span>
          </div>
        </template>

        <header>actual model</header>
        <div id="model"></div>

results in

static path [[test.0.value]]
[0] 3
[1] 2
[2]
dom-repeat
[0] =
[1] = 0
[2] = 5
actual model
[{"value":4},{"value":0},{"value":5}]

http://plnkr.co/edit/RFWLVhQ5yZ5E4sQYpLJG?p=preview

@arthurevans
Copy link

Looks to me like this.set('array.index.prop') is not notifying the dom-repeat, and the splice operators aren't notifying on the static paths. And unshift is apparently just confusing everyone.

@arthurevans
Copy link

Help us, @kevinpschaaf, you're our only hope!

@kevinpschaaf kevinpschaaf changed the title bindings not working bindings to individual array elements not working Jun 12, 2015
@arthurevans
Copy link

Re-running the plunkr sample against your branch, it looks like the dom-repeat is now registering correctly, but the static bindings are still wrong:

http://plnkr.co/edit/M6BodZU902ZLlCesVKEL?p=preview

@kevinpschaaf
Copy link
Member

@pvmart As I mentioned on #1839, the root cause of that issue plays into what you were seeing here. However, I didn't quite grok that you were binding directly to array indicies in your example. The binding system does not support updating bindings to explicitly de-referenced array elements like this (what you're seeing is a side-effect of how we uniquely key array elements and send path notifications of which keys change, rather than which indicies get updated for better performance, so the 0 in test.0.value is interpreted by the observation system as a key, not an index, and what results is actually correct per the keys you indicated, but that's clearly not what you wanted). We may put supporting this on the roadmap, but it may not be something that we prioritize soon.

However, the binding/observation system exposes enough hooks for you to do this manually (i.e. what we're lacking in the system is just the ability to set up what I describe below implicitly based on inferring that you are binding to array indicies, but it's fully possible to wire it up yourself).

You can write bindings that use an inline function to help dereference the array and keep the value in sync when the array mutates, like {{arrayItem(test.*, 0, 'value')}}, where arrayItem is defined like this:

arrayItem: function(change, idx, path) {
  return this.get(path, change.base[idx]);
}

The test.* dependency to arrayItem causes it to be called for any updates to the array, and it then uses the get helper function to dereference the array with the index & path specified in the binding (note I'm using the optional 2nd arg to get to provide a different root from which to dereference the path).

Your code would then look like this:

        <div>[0] <span>[[arrayItem(test.*, 0, 'value')]]</span></div>
        <div>[1] <span>[[arrayItem(test.*, 1, 'value')]]</span></div>
        <div>[2] <span>[[arrayItem(test.*, 2, 'value')]]</span></div>

I updated your JSBin below using the #1855 branch, so once that fix lands you should be able to use this workaround.

http://jsbin.com/nasafe/3/edit

@arthurevans
Copy link

Thanks @kevinpschaaf -- it looks like I've got this wrong in the docs.

https://www.polymer-project.org/1.0/docs/devguide/data-binding.html#path-binding

I'm somewhat confused by the distinction between keys and indices here, and exactly how we should communicate this distinction. But it sounds like the short version is:

  • associative-array/hash style access: use {{record.field}} in place of (0.5-style) {{record[field]}}
  • standard array access: use a helper like arrayItem, above.

@pvmart
Copy link
Author

pvmart commented Jun 13, 2015

With 1839-1854-kschaaf branch changes made by push, pop, shift, unshift not propagating to dom-repeat if called asynchronously.

http://jsbin.com/kiwacasiju/1/edit?html,output

Fix: https://github.com/pvmart/polymer/pull/1/files

@kevinpschaaf
Copy link
Member

Thanks-- I'll take a look at that. There were a few other issues with the branch that I'm working out, so it will be a little longer.

@kevinpschaaf
Copy link
Member

Root cause here w.r.t. notifying based on individual array element changes was fixed in #1854.

Note related issue opened here: #1913, regarding problems when primitive values are duplicated in the array.

kevinpschaaf added a commit that referenced this issue Jun 22, 2015
- Fixes bug in previous fix for #1839/#1854 where notifying properties would not notify during configuration
- renames `_setPropertyQuiet` to `_setProperty` with `quiet` arg
- renames `_propertySet` to `_propertySetter` for clarity
@kevinpschaaf kevinpschaaf changed the title bindings to individual array elements not working setting individual array elements not working Jun 24, 2015
@mikkokam
Copy link

Testing:

this.set("list.0", {name: "changed a"});
this.set("list.1", {name: "changed b"});
this.set("list.2", {name: "changed c"});

results in dom-repeat to be updated now correctly.
BUT: Running this after the changes:

this.push("list", {name: "a new one"})

results in list.1 and .2 to stay changed, but list.0 is reset to the original value.
Why is this?

Demo:
http://plnkr.co/edit/lNVG2dDmJtuf7hh8Fxf3?p=preview

@mikkokam
Copy link

OK, might be this
https://github.com/Polymer/polymer/blob/master/src/standard/notify-path.html

 if (array) {
            var coll = Polymer.Collection.get(array);
            var old = prop[last];
            var key = coll.getKey(old);
            if (key) {   // THIS ONE
              parts[i] = key;
              coll.setItem(key, value);
            }
          }

path.0 works if you change

if (key !== undefined)

@Alvarz
Copy link

Alvarz commented Dec 24, 2015

nothing here works anymore

@zoechi
Copy link

zoechi commented Dec 24, 2015

@Alvarz Do you expect any response to your comment?

@Alvarz
Copy link

Alvarz commented Dec 24, 2015

@zoechi hi! excuse me that behavior it's just i was really really mad because i'm getting big issues with data binding and all the post or guides or answer only work with version 0.5, i really dont expect any answer and again I really apologize for my behavior.

@zoechi
Copy link

zoechi commented Dec 24, 2015

@Alvarz :D I suggest asking on StackOverflow about your actual problem, ideally with a Plunker/JSBin that reproduces the issue.

@Alvarz
Copy link

Alvarz commented Dec 24, 2015

@zoechi Thanks I will do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants