Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Could we get a component binding option of '>' for one way binding up to a parent? #15902

Closed
1 of 3 tasks
sonicblis opened this issue Apr 7, 2017 · 21 comments
Closed
1 of 3 tasks

Comments

@sonicblis
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:

Right now, the only option for passing values out of a child component to a parent is the '=' two way binding which everyone agrees should be avoided. Parent values will overwrite child values in this case.

Expected / new behavior:

With a '>' binding, an author could explicitly communicate that the child is going to be providing this value and whenever it changes, it should be set to this parent property. You also avoid the performance hit of the two way binding.

Minimal reproduction of the problem with instructions:

This fiddle gives an idea of what I'm suggesting.

Angular version: 1.6.3

Browser: all

@Narretz
Copy link
Contributor

Narretz commented Apr 7, 2017

We recommend that you use & ( provides a way to execute an expression in the context of the parent scope), which is more verbose but gives you more flexibility. I don't think a new > binding is necessary. It also fits better with the idea that children raise events to which the parent reacts, whereas the parent pushes bindings down to the child.

@sonicblis
Copy link
Author

sonicblis commented Apr 7, 2017

What I implemented in the fiddle is somewhat the opposite of '&'. Instead of the parent providing an expression to the child, I want the child to be able to provide a callback pointer to the parent without using two way binding. A key difference is that then the parent can trigger a child defined function without having to know any of the internals of the child, and we can get further away from having to use $watch.

I realize there are currently ways to accomplish what is in my fiddle (since it's functional), but I believe there's room for clearer intent when creating a component binding structure that can communicate that a value is intended to be provided to the parent context from the child which currently doesn't exist.

@sonicblis
Copy link
Author

sonicblis commented Apr 7, 2017

As another example, writing a filter/data display set of components could be as easy as something like this without the need for $scope.$watch in the child component or two-way binding.

angular.module('newBinding', [])
  .component('filterableData', {
    controller: function() {},
    template: `<div>
                 <select ng-change="$ctrl.triggerQuery(selectedSource)" 
                         ng-options="sourceKey in $ctrl.sourceKeys" 
                         ng-model="selectedSource"></select>
    	         <data refreshItems="$ctrl.triggerQuery"></data>
               </div>`
  })
  .component('data', {
    bindings: {
      refreshItems: '>',
      items: '<',
    },
    controller: function(resource) {
      this.$onInit = function() {
        this.refreshItems = (which) => this.items = resource.query(which);
      };
    },
    template: `<div ng-repeat="item in $ctrl.items">{{item.displayName}}</div>`
  });

@gcaaaaaa
Copy link

yes

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 10, 2017

So if I'm looking at this correctly, you want your child components to expose an API in such a way that a parent can call it and execute the component's internal code to, in this case, update its content, right?

I don't really see the benefit over using items as the only binding (input binding) and update the array from outside the component when necessary. This API has no need to be included in the same component that's used to visualize them.

When using a unidirectional data flow together with some kind of smart and dumb components I don't think this even has any use-case (or at least not one that necessarily needs this new binding). But feel free to correct me if I'm wrong here!

From a reusability point of view, you'll want a component to represent the items (only have an items binding) which would be known as a "dumb" component (one that is only responsible for visualizing state and has no knowledge of where the data is coming from). You'll also want a parent component whose responsibility is to provide the data to its child (this one is smart, it knows where to retrieve the data from), so this also means being responsible for refreshing.

@Narretz
Copy link
Contributor

Narretz commented Apr 10, 2017

@sonicblis I see what you mean now, but I tend to agree with @frederikprijck that this is not something we actually want people to do. In your filter example, I'm not sure why the filter component should be responsible for resetting te data, if you can just pass in an array from the parent.

@sonicblis
Copy link
Author

sonicblis commented Apr 10, 2017

@frederikprijck, thanks for thinking this through. I should have left out the items: '<' binding to make the example more clear that the component isn't meant to be a dumb component.

With your solution (moving data access higher), I can't ever have a child component that's also a smart component (that does display and has parent triggerable functionality) without continuing to $scope.$watch (or $broadcast =\).

I find myself solving this problem with a $scope.$watch on some arbitrary binding and running logic in a child component in the $watch function. If, in general, I had two one-way bindings (one in '<', and one out '>'), I could be done with $scope.$watch in child components I think.

As one more simple example, if I have a graph component that displays a common roll up of data, and I want to use it multiple times, I could avoid every parent being responsible for data access, but instead encapsulate data access in the graph component itself with the ability to trigger a refresh from any parent if necessary.

Thanks again for looking at this. Happy to contribute a PR if there's any hope.

cc: @Narretz

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 10, 2017

With your solution (moving data access higher), I can't ever have a child component that's also a smart component (that does display and has parent triggerable functionality) without continuing to $scope.$watch (or $broadcast =).

That's incorrect, I don't see why that would be impossible. You could perfectly move the data-access in the dumb component I mentioned, making it both responsible for data-access and visualisation (not a bad thing, just not that dumb/reusable). You can, if you want, later abstract the data visualisation part into a dumb component (but the data-access stays out of the dumb component).

But that's about the new feature you're proposing, I personally don't see any use-case yet.
About what you're trying to do, what about using something like:

angular.module('newBinding', [])
  .component('filterableData', {
    controller: function() {},
    template: `<div>
                 <select
                         ng-options="sourceKey in $ctrl.sourceKeys" 
                         ng-model="selectedSource"></select>
    	         <data which="selectedSource"></data>
               </div>`
  })
  .component('data', {
    bindings: {
      which: '<'
    },
    controller: function(resource) {
       this.$onChanges = function(changes) {
         if(changes.which) {
            this.items = resource.query(this.which);
         }
      }
    },
    template: `<div ng-repeat="item in $ctrl.items">{{item.displayName}}</div>`
  });

Here's a modified plunkr to demonstrate it: https://plnkr.co/edit/qb1NCLSeZgGPuRVDqnpg?p=preview

@langdonx
Copy link

@frederikprijck $onChange doesn't do deep watches, so we have $watch for that, but $watch won't trigger when nothing has changed (a re-query). This is a common problem I run into developing components and why I thumbs'd up 4 posts ago.

Your example works and all, but outside of Angular demos, I feel like allowing binding like that to flow through as DOM changes is an anti-pattern.

https://plnkr.co/edit/PzNasQKuWVe0G4K4RfZw?p=preview

Here I've taken your example, added an extra field, and put a search button on it. I've got skip/take already set on my search object, so when the user clicks the search button, I'm only changing the properties that need changing. This is good because we haven't fired any unnecessary watchers as the user builds up their search criteria. Unfortunately if you try to search for the same thing twice, the search object hasn't changed, so nothing happens. Imagine searching the same value on Google but the second time nothing happens. 😢

So you're forced to re-create the search object or set some other property on it to like changed = Date.now() so that the $watch will fire, which isn't very intuitive.

@sonicblis
Copy link
Author

@langdonx, not to mention the search that will take place with default params since the $watch will fire once initially. Imagine if google did a search the first time you opened it but before you provided a search string.

@frederikprijck, $onChange and $watch is what I'd like to avoid in my child when not necessary. My main point is still that there is a difference between a child component responding to a change in the parent and a parent being able to explicitly read child data without unnecessary two-way binding (up to this point, so that the child can provide a function to the parent to call explicitly, but the intent is for a more global capability of child to parent one way binding).

Here's another example of where it would be nice to have a '>' binding as a form is providing edits on a provided object, but the parent is triggering the action to take on the edits:

https://jsfiddle.net/sonicblis/p630v7uf/3/

And then here, where the child is running some complex logic before returning a complete and valid edited entity.

https://jsfiddle.net/sonicblis/p630v7uf/4/

All doable with the current bindings, but would be better with one-way binding from child to parent. It seems like such a win to be able to define "out params" in the child's interface that you can then access from the parent.

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 10, 2017

@langdonx

So you're forced to re-create the search object or set some other property on it to like changed = Date.now() so that the $watch will fire, which isn't very intuitive.

It might not feel very intuitive in the beginning, but it's not bad from a performance point of view to use immutables as it eliminates the need for deep watches.
Angular's DI is based on object referencing so looking at upgrading, re-creating the object isn't that bad either.

@sonicblis I'm not a fan of two-way binding, but I wouldn't really structure components as how that fiddle does. If you have the need to implement your components that way (have the save button outside your component and retrieve the child component's state when necessary) I think it's what two way binding was introduced for. It makes the fiddle do the same with less code: https://jsfiddle.net/s8csjnyn/1/ .

The second fiddle is a bit harder to implement that way. Components are generally push based, not pull based. So pulling state out of the component feels unintuitive.
Why nog make use of a directive/component using ngModel to add the necessary validators/parsers to ensure the complex business logic/validation is applied to the editedPerson (this would also be a two way binding, and not result in a function you can call from a parent)?

Moving the complex business logic outside the inner component but inside the save handler should also remove the need for the > binding.

@Narretz
Copy link
Contributor

Narretz commented Apr 11, 2017

There's a PR for something that achieves this differently: #14080 With this, you can publish a component into its parent scope. This is very useful for sibling communication, but could also be used for parent - child communication. It would not be enforced by a strict interface, though.

@sonicblis
Copy link
Author

The ability to avoid two way binding seems like it would be enough of a gain. Seems like there's no trade off for the benefit. Also, I'd hate to put my complex business logic in every parent that's going to use that editor when I could nicely encapsulate it in the editor. I don't get the downside. Can you help me see how this is actually a negative instead of how I can make due without it?

I'm not suggesting that there's a need for '>'. I'm arguing it would be nice and consistent with existing patterns. Is there any chance a PR would be merged with a solid implementation?

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 11, 2017

@sonicblis Only taking Angular into account, I wouldn't say merging this is a good idea (not saying it won't get merged tho) unless Angular has comparable solutions. We'd be introducing an un-upgradable feature if Angular is not planning to implement this as well.

It doesn't sound a lot like how (web) components are to be used either.

Also, I'd hate to put my complex business logic in every parent that's going to use that editor when I could nicely encapsulate it in the editor.

Nobody is asking you to do that. If you currently have your editor as a single component (<my-editor>), you'd divide it into two now: The component purely for visualization (<my-editor-internal>) and the one for your BL (<my-editor>), including the one for visualization. This doesn't change anything in usage throughout ur application (the outer directive's name hasn't changed so refactoring shouldn't be that complex), but it does change your component structure and makes your application use a more component-based architecture which we try to encourage.

I guess it depends on what kind of BL. Looking at your BL it looks like it's some logic that's being applied to the object before using it outside the directive, this sounds a lot like what a $parser is used for. If it includes validation checks, I'd look into $validators, both existing for those purposes.

Imho there's 3 solutions to solve this with current AngularJS setup:

  • use two-way bindings as you're doing (I dislike)
  • use ngModel (which is a two-way binding anyway, so probably falls under the 1st one aswell) but it allows you to add a $parser or $validator.
  • refactor your components so you have no need for the two-way bindings.
  • Maybe there are even ways to solve this by using controller communication? (not sure if it is and if it would even make sense tho).

@langdonx
Copy link

langdonx commented Apr 11, 2017

@frederikprijck

It might not feel very intuitive in the beginning

None taken, don't worry about it. 😉

@Narretz Yep, ngAs sounds like it fulfills the same desires. It was submitted over a year ago though -- do you think it will land? Earlier in this conversation you said this is not something we actually want people to do -- is this one of the reasons why ngAs has sat for so long?

ngAs vs what is proposed here feels similar to the OO reusability argument about asking for the banana, but getting the gorilla holding the banana (something something whole jungle). Exposing the entire component controller with ES2015 classes will get you the controller and all of its dependencies. This seems like a code smell to me, but might be fixed when private state lands in ES20xx. I do love the use case of displaying a tool tip, which would work very well with what's proposed here (but works just as fine with '=' to be fair).

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 11, 2017

@langdonx

None taken, don't worry about it.

You either missed a part of your reply, or I'm missing what you're talking about. 😕

@langdonx
Copy link

@frederikprijck Sorry, just messing around. Was trying to express my disapproval of your implication that I'm new to this. ☹️

@frederikprijck
Copy link
Contributor

frederikprijck commented Apr 11, 2017

@langdonx I had no intention to call you new at all (other than the unintuitivity of immutables). You said it feels unintuitive to re-create the object. I wanted to state it feels unintuitive at first (so when recreating the objects for the first time for these purposes) but has some benefits in the long run and might feel a lot less unintuitive at the end.

@sonicblis
Copy link
Author

I'm familiar with $validators and $parsers, but I appreciate the suggestion.

It doesn't sound a lot like how (web) components are to be used either.

It seems like the closer we can get to components having a well defined interface, the better, so I'm not sure I agree with this being against the goals of components in general.

The lack of upgrade path to Angular 2 is a great point. Looks like it's just a whole hog exposure of controllers in that world which is a little surprising, but gets the job done. I echo Lang's ask about the PR for ngAs. Any chance that's getting merged some time soon?

@gkalpak
Copy link
Member

gkalpak commented Apr 11, 2017

This thread has grown long and I am not sure what all the arguments usecases are, but I will try to answer/give my view on some of them:

Right now, the only option for passing values out of a child component to a parent is the '=' two way binding which everyone agrees should be avoided.

As already mentioned, the recommended way to pass value one-way to the parent is the & binding. We have also briefly discussed providing something that is similar to Angular's @Output (I ironically proposed the > symbol for that 😁), but nothing concrete yet.

Parent values will overwrite child values in this case.

The way I understand it, this is irrelevant to the problem you are trying to solve, which is "who sets the value on the parent" (not the child). So, even in the > approach that you suggest, we can't prevent someone overwriting the value on the parent.

You also avoid the performance hit of the two way binding.

There is still a performance hit (for one-way watching). If performance is a concern, the & (or a new Angular-like output) should be the way to go, since it doesn't involve any $watchers.

I think the main issue with what you suggest, is that enabling child-components to modify the parent at will (even a specific property only), removes much of the declarativeness that we know and love in AngularJS. This is quite different than letting the parent modify the child, because a child can have only one parent, the opposite is not true (one parent --> multiple children).

A solution to that, is to still let the parent be in charge. This is achieved by Angular's outputs (where the parent has to explicitly subscribe to emitted output values and handle them as it wishes) and AngularJS' & bindings, where the parent is in charge of treting the passed in values the way it wishes). Again, this is not about what is and isn't possible; it's about how easy it is to tell what happens by looking at the parent component/template.

All than being said, I don't think there is something that is not possible with the current API. It's just more verbose than necessary/desired.

I understand that currently the child-to-parent communication story in AngularJS isn't great and requirs a bit more boilerplate than we'd want it to. But as already mentioned, I don't think it is wise to introduce new features that do not map closely to Angular semantics as this will make upgrading more difficult. Even more so, when there are alternatives that are more declarative and more in-line with "how Angular does it".

WRT to previously mentioned alternatives:

a) An Angular-like, Eventemitter-based "output" binding.
b) The ngAs directive in #14080 (which probably needs a new name 😛).

Besides, being more similar to Angular, (a) won't be much different than & in terms of reducing the boilerplate for example. So, while a good change in general, it won't add something wrt the usecases discussed here.

(b) is similar Angular's template-local refs and would also make it easier to access child methods from the parent (in some usecases at least). The problem with ngAs (and probably the reason it hasn't landed yet) is that (in contrast to Angular) AngularJS is not able to restrict a ref to the local template, so we must consider the implication more carefully and be more detailed in the documentation of how this feature should or should not be used.

The bottom line is that I don't think the proposed feature is a step in the right direction, but we should work on alternative approaches that are more "upgradable" and declarative.

@sonicblis
Copy link
Author

I understand that currently the child-to-parent communication story in AngularJS isn't great and requires a bit more boilerplate than we'd want it to.

That nails it. I appreciate the discussions and look forward to that friction being slightly reduced if that's what the future holds. =]

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

No branches or pull requests

6 participants