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

Binding to array element fails if members are base types instead of objects #444

Closed
jsobell opened this issue Jul 3, 2016 · 22 comments
Closed

Comments

@jsobell
Copy link

jsobell commented Jul 3, 2016

@darkelyte posted this as an issue:
https://gist.run/?id=ce4f511fd3c40cda9f93c05c4f869e52

export class App {
  items = ["first", "second", "third"];
}
  <ul>
    <li repeat.for="item of items">${item}</li>
  </ul>

  <p repeat.for="item of items">
    <input value.two-way="item"
  </p>
@EisenbergEffect
Copy link
Contributor

I doubt that is fixable. It's not even clear what that should do. Perhaps @jdanyow can comment.

@jsobell
Copy link
Author

jsobell commented Jul 3, 2016

Do we not expect an array to be bindable on a VM?

@EisenbergEffect
Copy link
Contributor

Think about the code you are showing above. That wouldn't even work in JavaScript. An alternate approach is needed.

@araney
Copy link

araney commented Jul 3, 2016

I would expect that binding to array members would produce the same results as binding to object properties. The gist linked above shows that the code works as a one way binding, but updating the values on the inputs or programmatically via the button does not update the bound values.

@jsobell
Copy link
Author

jsobell commented Jul 3, 2016

Sorry, I must be missing something obvious here.
We have an array of strings 'items' off the VM, and the UI wants to bind to the first item of that array.
Why would you not expect ${items[0]} to work, and what might an alternative approach be?
It seems to me that this is an incredibly common scenario, hence why I'm asking if I'm missing something.

@jsobell
Copy link
Author

jsobell commented Jul 3, 2016

Sorry, I meant <input value.bind='items[0]'>

@jsobell
Copy link
Author

jsobell commented Jul 3, 2016

It's actually a bit more obscure than I thought. If you bind to items[0] then the changes are recorded, but the UI does not update, so replacing the existing example with value.bind="items[$index]" allows updating of the underlying array, but changes are not reflected back through the binding.

@jdanyow
Copy link
Contributor

jdanyow commented Jul 5, 2016

This scenario isn't valid.

export class App {
  items = ["first", "second", "third"];
}
  <p repeat.for="item of items">
    <input value.two-way="item"
  </p>

It's the equivalent of expecting this to update the contents of the array:

for (let item of items) {
  item = 'something else';
}

This...

  <p repeat.for="item of items">
    <input value.two-way="items[$index]"
  </p>

...worked in Chrome when we were still using object.observe. It only half works now because indexed assignment of an array is not observable using the means we currently have at our disposal unless we resort to dirty-checking.

@jsobell
Copy link
Author

jsobell commented Jul 5, 2016

OK, so we're saying that as the let...for...of syntax only acts upon values, we can't bind to them. That makes sense.
So when people ask how to bind an array of values, what do we tell them to do? Do we say they have to box them into individual objects?

@jdanyow
Copy link
Contributor

jdanyow commented Jul 5, 2016

I don't know- left this open so we could come up with a recommendation- here's a gist that shows what currently happens: https://gist.run/?id=1e9d861556a7a03a51ef7e1c9496e6de

(data flows from the view to the view-model but not in the opposite direction)

@araney
Copy link

araney commented Jul 5, 2016

As a user of the framework, while I wouldn't expect this to work:

for (let item of items) {
  item = 'something else';
}

... I would hope that the binding system would get around that limitation. If the answer is simply, "it doesn't", then so be it. Though I'd then like to open an enhancement request for this functionality if it is at all feasible.

@jdanyow
Copy link
Contributor

jdanyow commented Jul 5, 2016

@araney but that doesn't even work in plain javascript. Why would you expect that to work in the aurelia equivalent of for-of?

@jsobell / @EisenbergEffect 7d01567

@araney
Copy link

araney commented Jul 5, 2016

@jdanyow Maybe I have the wrong expectations of the framework, but I would expect it to work for the same reason I can expect updating the text in an input would change the value of a property in my javascript object.

When encountering the aurelia for-of, I didn't translate that to a vanilla for-of, but instead assumed it was merely mirroring the syntax of the vanilla for-of for ease of use.

@jsobell
Copy link
Author

jsobell commented Jul 11, 2016

@jdanyow @EisenbergEffect I think what @araney says is an important understanding issue for Aurelia.
We know that javascript has no concept of boxing variables to allow pass by reference, and that if we want to modify data items in a loop we would have to reference the source array by index.
There's also no question that we have to refer to the array element by index to allow updating of the original value, so we have to

  • to make sure the binding works two-way for bound array entities
  • document clearly in the data-binding section of the documentation with an explanation as to why it's possible to read values from the loop iterator but not write them

This isn't a technical issue related to Aurelia (except perhaps the current two-way binding issue) but will be a tripping point for people.

@EisenbergEffect
Copy link
Contributor

@jsobell Absolutely. I agree. @jdanyow is working on some new bindings docs. Jeremy, if you can take this into account and include some notes to this effect, that would help people.

@EisenbergEffect
Copy link
Contributor

@jdanyow One idea I had for the binding docs might be to include an extra article, beyond the normal explanatory content, that is more of a "recipes" article which just lists problem/solution combos. It would be easy for us to extend that over time as well. Just a thought.

@jsobell
Copy link
Author

jsobell commented Jul 11, 2016

@EisenbergEffect Absolutely, and I'd suggest 'Recipes' be added as a new left-menu item, as there are dozens of snippets and architectural practises / best-practises that new adopters would benefit from

@EisenbergEffect
Copy link
Contributor

We can add a grouping, similar to Binding and Templating, that is dedicated to Recipes. We can include various articles in multiple sections. So, we could have the binding recipes under the Binding group, but also have them appear under the Recipes group, for example. In that way, whether the reader goes looking for help about binding or goes looking for a recipe, they can easily find that content. We’re doing a bit of work on our nav content structure now, so when that’s done (which it almost is) this should be a piece of cake to implement.

On Jul 10, 2016, at 10:03 PM, Jason Sobell notifications@github.com wrote:

@EisenbergEffect
Absolutely, and I'd suggest 'Recipes' be added as a new left-menu item, as there are dozens of snippets and architectural practises / best-practises that new adopters would benefit from


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
, or mute the thread
.

@weifang993
Copy link

I encountered the same problem when converting an Angular 1.x application to Aurelia. Augular 1.x two-way binding worked fine for "item of list" if the list is an array of strings. It is a design issue for the framework. The "item of list" syntax should project a consistent behavior, no mater what the list contains. Developers should not have to use a different syntax simply because the list is an array of strings. As a developer, I am drawn to Aurelia for its simplicity and elegance in design. Things like this should be encapsulated by the framework.

@weifang993
Copy link

I obviously spoke too soon. Angular 1.x did not handle the issue. My apologies, but I do expect Aurelia handle this better.

@Alexander-Taran
Copy link
Contributor

Stale

@EisenbergEffect
Copy link
Contributor

Closing since this can't be made to work in the binding engine and we've added a lot more binding docs with examples.

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