binding
binding copied to clipboard
aurelia
Consider adding an option to control AccessMember's automatic object creation
AccessMember automatically creates an object when assigning a value to a property on an object that doesn't exist.
Some people love this behavior. Other people don't like it.
Maybe we can make the behavior configurable?
cc @EisenbergEffect, @danfma
Hmm. Yeah...I remember when I added that...a long time ago. We should definitely make this configurable. It's great for demos...but not sure it it's great for real world development. We should leave the default setting the way it is today, but enable to be swapped. Not sure what is the best way to handle settings like this. Recommendations welcome. @jdanyow You are the binding expert :)
To be honest, I really like this behavior except when it is used with the if attribute binding, because if I remove the inner nodes, I don't want the removed bindings of these dead nodes to continue to recreate things that are not there anymore.
But for the rest of the application is very fine!!
In terms of the alternate behavior, would you both agree that instead of creating an object the assignment should just fail silently, no errors thrown?
@jdanyow, did you understand that I'm talking about only the if binding? If yes, in my opinion, all inner bindings should be inactive or removed, so it will not affected the default behaviour and will work like knockout, for example.
@jdanyow in our application, we really need this, and it should be deep and not shallow binding, because we have very complex and dynamic nature of data and the way it should be bound and sometimes user needs to create it.
thanks for taking care of it.
@rob, this feature is very important in real world application. Let me give some valid argument that we do a million times every days since that we work with aurelia since few months. We are building a first class web content edition with aurelia which allows content authors to edit content. There are many ways to achieve that but the outcome is repeated code and not maintainable.
- The simple quick and dirty is to have a custom editor for each page layout ou component by naminf convention that activate the editor dynamically
- Defining a fake object as a dummy data as a preview
- Having a manifest like a schema to define your Poco
- We can generate the editor based by 3)
All of these can be an acceprable solution but how much effort to define such contract for any binding when you want to edit a content and persist it back to your server when you have elasticsearch or mangodb playing with....
If i may as we love convention and until this rule make the simplicity of aurelia, why not to detect array members when its name ended by ...Collection or ...Items? When you ask a front developper to bind data, he will never think about manifest nor a poco. Based on its binding, we help him to build his tree model and can be persisted naturally in json.
Leon
We will not remove the feature- we're only talking about adding an option to disable it.
@jdanyow, thank you for your support. There is a chance that by naming convention, an array or map can be initialized. ...Items or ...Collection, ...KeyValues This feature is very usefull and avoid to pre instantiate the property in an object hierarchy. Tks
Leon
@jdanyow also i must mention that this feature currently does not do deep and recursive binding, it is only one level deep, so it works on this model.name but not model.componentes.savedComponents so could you please look at that.
@leon-andria / @devmondo I don't think there's any plan to add further support for this "feature" for the beta. The intent of this issue is to add an option to turn the existing behavior off for those that want more explicit/predictable behavior.
I agree with @danfma, the only problem here is the combination of this feature with if.bind, show.bind, and ternary ? : operations. I like the feature in general, but it creates odd behavior with if and show.
An option to turn off existing behavior, may work but I don't think it would be very intuitive. For my issue aurelia/templating#83, I don't think it would occur to me or a common user to "turn off the automatic object creation feature". As a result unless the option's syntax is very clear and intuitive, you will have to keep on closing github issues and pointing people to use this option.
Alternatively, is there a way these automatic objects can evaluate to false when coerced to a boolean? I believe that would solve the above issues.
The problem manifests with if.bind but it's not the if binding command that's causing the problem, it's the input value binding to a path expression whose object is null.
I agree with @akircher's concern that people will have a difficult time figuring out there's an option they need to turn on to avoid this behavior. I would love to make the default behavior be "don't automatically create objects" and have an option that turns on that behavior. I think that would be most intuitive- people would need to opt-in to the magic.
Taking things a step further, it might be better to turn of the behavior entirely and don't add an option for it (because it will cause problems in situations where a plugin expects the behavior to be turned off and the app has turned it on). Instead maybe we could add a binding behavior that people can use in specific locations where they want automatic object creation.
This is part of the reason it's taken so long to get this issue resolved- I've been a little conflicted as to how it should be done. The binding behavior approach didn't occur to me until just now.
Thoughts?
I like the idea of the binding behavior to opt in. Can you list out here some sample code for the different scenarios where we would and would not want the automatic creation? Seeing those altogether would be helpful. If you can create the psuedo code in terms of your proposed binding handler, that would be optimal :) I'd just like to get a feel for it from the developer's perspective.
In ecma script discussions, there was a thought of adding an "existential operator" such as deeply?.nested?.field which would allow the user to opt into the behavior and be eventually/possibly consistent with javascript syntax.
That's in interesting idea. Did they do something like that for C# recently. @jdanyow I think this may be something worth considering.
Bases with our expériences, having the option off/on is not really helpful for the développer because it requîtes so much dependencies between plugins on your on apps. This is really dangerous for application design and sustainabilities. Today having it off is painfull because the if.bind syntaxe polluate your html code for visibility but this choice is douable to make it off. I would prefer on but this means that the binding engins takes more responsabilities and may ne confusing for object type création. A trade off option is really using a design pattern of délégation or contexte. Having someone to validate a model with schéma would be very usefulll and help the engine to create of the object instance or to delegate the création.
I think the "optional chaining" / "null propagation" spec only covers accessing properties. Aurelia's binding system already matches that behavior when accessing props on objects that don't exist.
The issue here has to do with property assignment- a few developers are defining their object structure in their HTML templates instead of creating the object structure ahead of time and binding to it. To be clear, we're not talking about binding to undefined properties here, we're talking about binding to properties of objects that are not yet defined or null- eg value.bind="undefinedObject.someProperty". Aurelia's behavior is not consistent, for example, value.bind="undefinedObject.someProperty" does not work the same as value.bind="undefinedObject['someProperty']". The former has magic object creation and the latter does not.
I haven't used the automatic object creation in any apps but I have had to work-around it... in other words I'm probably not the best advocate for this feature :stuck_out_tongue_winking_eye: Definitely need some feedback here!
Here's some pseudo code for the binding behavior approach:
Without automatically creating objects:
<input value.bind="foo.bar.baz">
Opting in to automatically creating objects:
<input value.bind="foo.bar.baz & auto">
Opting in to automatically creating objects and providing an object factory:
<input value.bind="foo.lorem.ipsum & auto:factory"> <!-- factory is a function that accepts an AccessMember expression and returns an object -->
I'm not too sure about the name of the behavior... auto is pretty ambiguous. Also not sure about the optional factory argument...
@jdanyow Yes, you are right the spec doesn't handle this specific need. My thought is just that I think a new type of syntax here might be more intuitive than trying to solve this issue with binding behaviors.
<input value.bind="foo.bar.baz">
Throws error if you try to get/set baz when foo or bar are null.
<input value.bind="foo?.bar?.baz">
Enables null property access and object creation when foo or bar are null.
Edit: You also get an extra degree of specificity, since you could do foo.bar?.baz which would not be possible with binding behaviors (although not sure if that is actually solving a need or not).
@jdanyow: I like the idea, for a big app, it will be hard to governe rules for this. By keeping your idea, rather having auto:factory, it will be good to have a chaining pattern handler
@akircher I like the specificity your proposal enables but throwing when accessing foo.bar.baz when foo or bar is null or undefined would be a big change in behavior.
A lot of people are getting tripped up by the automatic object creation, there are currently 8 issues that have been closed as duplicates of this issue. I was hoping we could turn it off while providing a path forward for those that want the binding system to instantiate objects. The binding behavior approach would accomplish this. Not as flexible as your approach but less disruptive. I was thinking the number of situations where someone wants/expects the automatic object creation is minimal, so there's no need to build a ton of support for this into the binding engine.
Thoughts? Am I way off base? I haven't been using this feature so my point of view may not match up with everyone else's.
@jdanyow To make sure I understand the scenario. Is it correct to say that the only time you might want the automatic creation to happen is when you are binding from the view into the view model from a form control and part of the binding path doesn't exist?
For our case, it's always on form control for editing (editor views). A display view doesn't this kind of stuff.... So, make it off and binding behavior proposal with auto:factory will make happy everyone.
Our plan is to follow the recommendation of @jdanyow to remove the functionality that auto-creates instances and instead add a binding behavior that enables it. Since this will be a breaking change for some developers, this will not be released until we release Beta 2.
here's how you can disable automatic object creation in the meantime:
import {AccessMember} from 'aurelia-framework';
AccessMember.prototype.assign = function(scope, value) {
let instance = this.object.evaluate(scope);
if(instance === null || instance === undefined) {
return;
}
return instance[this.name] = value;
}
https://gist.run/?id=2c87ea6a166cef3c1e0e
Thanks to all. Sounds like a good plan. In the mean time, another workaround is using something like this in the template value.bind="(nullOrObject || {}).property"
@EisenbergEffect, If you remove the auto-creates instances by default, it will make many of us really sad. I believe that to disable by default is a better approach. For a community adoption, it would be good that you do a Uservoice to know the most demand of developpers. We know that nothing is perfect but when you have to deal with many big applications to maintain, we are not going to play around breaking changes as we already did with these topic after each releases.
regards,
Isn't that what we said? Auto-create would be disabled by default but you could use a binding behavior to enable it on a particular binding. Am I misunderstanding?