-
Notifications
You must be signed in to change notification settings - Fork 27.4k
One way binding for objects in directive #12835
One way binding for objects in directive #12835
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
Is this usecase common enough to warrant further complicating the DDO API ? |
@gkalpak well this is one point I want to discuss, but I don`t see why we should replicate a certain piece of code all over when we can just let the framework do the job for us. I'm happy to hear any other thoughts on the matter. |
The thing is that there are too many things that we can make the framework do for us.
So, before adding a new feature, one has to weight (1) against (2) and (3). Back to my original scepticism: (Points (3) and (4) are not really relevant in this case, so fcusing on (1) and (2).) I feel that the added feature is not common enough (i.e. not many developers need it/will use it), but it will add to the complexiy of the DDO (i.e. people will have to understand what But, let's see what other people think. They might find it a useful/common enough feature, to make it worth the complication of the DDO. @aoancea, in any case, it is awesome that you took the time to put this together. 👍 |
I agree with @gkalpak I do not find the use case common. An alternative would be to create a new directive that does this for you. Eg oneWayBind and would work |
@gkalpak very nice justification. I totally agree with all of the points you mentioned in there, that's why I even wanted to discuss about it. Probably I should have raised an issue on the matter, but having the code in front of us, as we are all developers, I thought that it would be easier to understand the problem and talk on the matter. Also thanks for appreciating my contribution, and if you have any points of attention please share as I`m new to this contribution stuff. @lgalfaso I thought about doing that at the start, but adding a new operator seemed more interesting. I |
I think there may be some benefit in adding this in the case where we want to move more toward a single direction of data flow - properties down and events up. This would be beneficial to the ng-forward project. |
Seconded what @petebacondarwin said. We currently add one-way binding via a kind of hack. It would be nice to just use an official feature to get this functionality. |
There is a lot of value in a one way operator for components that have similar behavior to Angular 2 components (date flows down, events bubble up). If you wanted to build an entire Angular 1 app this way, it wouldn't be feasible to use a @aoancea: I can take on getting this finished up if need be! |
@MikeRyan52 I already solved the problem with what @lgalfaso suggested. Here is the link to it: http://computerscienceunveiled.blogspot.ro/2015/09/angular-modal-directive-with-one-way.html What I'd still do is replace the hard-coded operators with some configurable ones. Those 3 that are now should be included by default, but you should have the possibility of adding your own. In this way we could have extensibility at a very low cost. I'm thinking of a way in which interceptors are added now. |
My experience is that the need for one-way binding is more common than two-way especially for directives. We try to keep our watches to a minimum and for most scenarios we don't want a directive changing the model it was given. Our directives offer a means of passing objects to them via binding and that can only be done with two-way or '&' binding. Two-way binding is a problem because you end up with two watches, one of which isn't needed. '&' binding is a problem because of the semantics involved. I'll add another example that may help with motivation. We have built various directives that take an array of objects given to it by the consumer. Two-way binding doesn't make sense here because the directive will never alter the array. So we're left with using '&' binding which then puts a rather strange requirement on the consumer to give us an expression to deliver the array. Having the option to specify one-way binding in this case would solve this and many other scenarios we have as most data in our application flows into the directives and only flows out through events. Both one-way and two-way binding would look the same from the consumer standpoint. vs. '&' binding which has poor semantics. $0.02 |
I just wanted to mention that one-way-bindings, won't prevent a directive from altering the data (unless you pass primitives or immutable/frozen objects). |
@gkalpak that depends upon how the feature is implemented, no? If we always took a copy of objects in a one-way binding watcher then we could modify the inner version to our heart's content without affecting anything in the outer scope, which I believe is the result that we would want from such a feature. I am still keen to see this happen but I appreciate that it would need to be thought through carefully to prevent unwanted loss of performance, etc. |
Good point @gkalpak. I suppose whether or not the data is altered by a directive is more of an implementation detail although by adding support for one-way binding allows the responsibility to be communicated more effectively. This could also create an opportunity for the directive to work on a copy of the provided binding data. A copy would allow data to be in the states of original and working with the flexibility for the consumer to commit the working copy to the original based on some event or state. |
That wouldn't be so straight-forward, I think. For copying to make sense as a way to prevent modification, it needs to be a deep copy and deep copying arbitrary objects isn't a solved problem :) TBH, I don't think we can get away with copying the object, unless we set some hard rules about the kind of properties that are allowed on such an object (and its "sub-objects"), which would make the feature kind of restrictive. |
What is the Angular 2 strategy in this regard? |
They pass the object as is, so if you change a property then the change is reflected back to the parent. |
So the key point is that we don't make a copy but we do only watch the outer value for (identity) changes and don't overwrite the inner value unless the outer value (identity) actually changes. |
Sounds reasonable. I just think some people will confuse one-way binding with unidirectional data flow, so we should make it clear (e.g. in docs) that it's not the same thing 😃 |
BTW, @robstove, looks like you're missing the fact that the attributes for <data-list get-items-source="getMyArrayOfObjects()"> The term 'expression' doesn't mean it has to be a function call. This works fine too: <data-list get-items-source="myArrayOfObjects"> However, these bindings don't setup watches at all whereas it's usually desirable that a component react to changes in its inputs. |
@thorn0 Yes of course. Its a problem of semantics. If I use an expression I would expect the semantics of the expression to look like function rather than a property because we are evaluating something. It's not that we can't do these things it's a matter of doing things in a way that make them easy to communicate. |
@robstove I read your "from the consumer standpoint" like you meant the consumer of the directive, the one who uses it and never looks into its source code. For such a person, |
Sounds right. It should work totally like |
They're not the same, but the former is needed to implement the latter. |
I'm not sure it is. Unidirectional data flow is more of an app implementation decision that a framework feature. AFAICT, one-way binding isn't necessary for unidirectional data flow. |
Perhaps I should have put it like this: it's convenient to use the former to implement the latter. Implementation decisions are uh... implemented using framework features, aren't they? And a framework can be more friendly to some decisions and stand in the way, to a greater or lesser extent, of implementing others.
Yep. Good old two-way bindings can be used too. They just require more discipline. |
Not necessary, but somewhat dumb without it. I've currently got a unidirectional application and if it wasn't for disciplined immutability on my part I'd be two-way binding to all kinds of things I don't want to change. It was overly tricky to set up a unidirectional data flow with two way binding via '='. |
I dont really see how this will change with one-way binding. Let's assume one wants to avoid having child components mutating data that is passed to them from a parent component. Let's also assume that
Now, with one-way bindings, only the second way of modifying parent data from the child component will be prevented. And it will be prevented in a (nasty imo) "silently fail" way: The changes will be visible in the child component only, until (based on the implementation) some event makes the parent data overwrite those changes in the child component. IMO, one-way binding requires even more discipline to implement unidirectional data flow. And there is also the caveat, that people might misinterpret one-way binding and get a false sense of "unidirectional-data-flow-ity". I'm still in favor of adding one-way binding, because it can reduce unnecessary watchers in certain situations where it's more appropriate than two-way bindings, but there are several things to take into careful consideration and properly communicate to devs. |
This change allows the developer to bind an isolate scope / controller property to an expression, using a `<` binding, in such a way that if the value of the expression changes, the scope/controller property is updated but not the converse. The binding is implemented as a single simple watch, which can also provide performance benefits over two way bindings. Closes #13928 Closes #13854 Closes #12835 Closes #13900
While I was trying to build a modal with angular, i ended up with the problem of changing something in the modal pop-up and the changes were sync-ed with the original object.
Instead of doing all the work in each directive for copying the objects, i decided to extend the directive's operators to support one-way object binding.
I would like to also discuss about these changes if they are between the guidelines of angular js so we don`t pollute it with unwanted code.
This was tested on version 1.2.16(code is a little bit different there) and code was adjusted for this version.
If this gets approved by the community, I`ll upgrade my project to the latest version and have it tested fully.