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

$onChanges does not trigger for 1-way binded resolved data #2888

Closed
pierrekraemer opened this issue Jul 25, 2016 · 22 comments
Closed

$onChanges does not trigger for 1-way binded resolved data #2888

pierrekraemer opened this issue Jul 25, 2016 · 22 comments
Labels
Milestone

Comments

@pierrekraemer
Copy link

pierrekraemer commented Jul 25, 2016

In the following example (also available in this plunker), I was expecting the $onChanges hook to be triggered when the object that is resolved for the consumer component is reassigned by the editor component but it does not. When using the modify method, the modifications are visible because 1-way binding is done by reference, but ideally, the consumer component would do a local clone of the object in the $onChanges hook so as to avoid any unwanted side-effect (like the component modifying the object).
Could someone explain me what is happening here ?
This second plunker is the same example, but using a service instead of a global variable.

let data = { name: 'initial' };

angular
.module('app', ['ui.router'])

.component('editor', {
  controller: function () {
    this.modify = (value) => { data.name = value; };
    this.replace = (value) => { data = { name: value }; };
  },
  template: `
    <input type="text" ng-model="value">
    <button ng-click="$ctrl.modify(value)"> modify object </button>
    <button ng-click="$ctrl.replace(value)"> replace object </button>
  `
})

.config(($urlRouterProvider, $stateProvider) => {
    $urlRouterProvider.otherwise('/');

    $stateProvider
    .state('root', {
      url: '/',
      component: 'consumer',
      resolve: {
        myData: () => data
      }
  });
})

.component('consumer', {
  bindings: {
    myData: '<'
  },
  controller: function () {
    this.nbOnChangesCalls = 0;
    this.$onChanges = (changes) => { this.nbOnChangesCalls++; }
  },
  template: `
    <p> value : {{ $ctrl.myData.name }} (nb onChanges calls : {{ $ctrl.nbOnChangesCalls }}) </p>
  `
});

For the record, I have set up a StackOverflow question on the subject.

@pierrekraemer
Copy link
Author

It seems that it is the fact that resolved bindings are 1-time binded and not 1-way binded that prevents the $onChanges hook to be invoked in the component when the resolved object is reassigned.
Should it be considered as a bug?
Would it be possible to do 1-way binding with the resolved properties?

@michalpanek
Copy link

I have the same problem with my app. And I don't now how to "fix" this problem.

All work good when you pass bindings in template like:

<child-component binding="$ctrl.someProprsToBind"></child-component> 

$onChange() work as should be when somePropsToBind change in parent-component. But If try pass binding to child component by resolve in route configuration it deosn't fire $onChanges() more than first time. I think it is serious problem with ui-router when you want to build app based on smart and dumb components approach, and you won't give any access to data to dumb/stateless components besides of this data which is sent through a bindings.

@bfricka
Copy link

bfricka commented Aug 11, 2016

This was answered in #2896. It's a very frustrating working with a 1.5 app that previously used ngComponentRouter, since you need to completely change how values propagate. Frankly, I think 1-way bindings would be nice.

@siddharth-pandey
Copy link

siddharth-pandey commented Aug 22, 2016

I've recently encountered the same issue as what @michalpanek has mentioned above. I've a smart component that consumes a service to get/post/put data and it has two different dumb components with input & output bindings defined. I use $resolve to specify the value for bindings in the template member. This results in $onChanges not getting called at all for any of the bindings - an object literal and primitive.

What is the workaround if this is not possible out of the box with ui-router? I'd like to use smart and dumb components approach but if there is any other workaround available, I can give it a try. I'm going for mix approach as I've need of two different states - a list and an details/editing view of an item - for which data is provided by smart component.

@pierrekraemer and @michalpanek did you guys managed to find any good solution for a mix approach of smart & dumb components?

@michalpanek
Copy link

michalpanek commented Aug 23, 2016

When you use ui-router for $onChanges I findind nothing:) and switched to ng-switch to render dumb components because of less problem with bindings updates, bindings send to child component, and with $onChanges also.

<div ui-view></div>

with some ui-router conf with states
replaced by:

<div class="someClasses">
  <div class="" ng-switch="$ctrl.someProps">
    <div ng-switch-when="SOMETHING1">
      <list-companies list-companies="$ctrl.listCompanies"></list-companies>
    </div>
    <div ng-switch-when="SOMETHING1">
      <company-item item="$ctrl.selectedCompanyItem"></company-item>
    </div>
    <div ng-switch-when="SOMETHING1">
      <modify-company is-adding-new="true"></modify-company>
    </div>
    <div ng-switch-when="SOMETHING1">
      <modify-company is-adding-new="false" item="$ctrl.selectedCompanyItem"></modify-company>
    </div>
  </div>
</div>

This code above replaces ui-view directive in smart component's template. There is no problem with bindings and with $onChanges. I'm not in 100% sure it is a good approach. I asked in Todd Motto github about that in his github.

@michalpanek
Copy link

I founded another, much more better solution for that issue:). Or, I think I found..

component conf:

app.component('grandChildComponent', {
  bindings: {
    name: '<',
    onEvent: '&'
  },

In ui-router conf:

.state('grandchild', {
      url: "/grandchild",
      template: '<grand-child-component name="$ctrl.nameForGrandChild" on-event="$ctrl.fireEvent()"></grand-child-component>',
    });

There is no use component: prop from newest ui-router.

There are bindings name, and onEvent. grandChild is stateless/dumb component, which get his bindings through ui-router conf in template prop in object configuration.

-onEvent binding can be fired in child stateless, and parent statefull component catch it.
-name is passed down from parent statefull to stateless child
-when binding name is changed in parent, then is changed in child also.
-$onChange() in child will be fired when binding will be changed in parent.

@bora89
Copy link

bora89 commented Sep 5, 2016

@michalpanek thanks for your solution, but could you please explain a bit more and provide full code? It is not that clear at the first glance, how you can pass fireEvent into child component

@michalpanek
Copy link

simple: there is no problem with passing down fireEvent to child component or other bindings, because parent component is already created( populated ? whatever) when ui-route render child component in the element with ui-view directive in parent component template. And because of that child component could get bindings from already existed parent.

I ge to that simple solution, when I got the thought that the ui-route state configuration object in angular config phrase is just "declaration" of state options, and this option object is use when ui-view is rendering this state, not in config phrase.

Bindings is passed to child component in the same manner from parent component in element with ui-view directive, like child component would be just a html element in parent component template, outside of ui-view directive.

This is simple plnkr (in es5) plnkr

@MystK
Copy link

MystK commented Sep 22, 2016

@michalpanek, your solution worked great and this is exactly what I needed.

@vbarinov
Copy link

@michalpanek thanks for the dumb state hack, it works!

@Josh68
Copy link

Josh68 commented Dec 2, 2016

@michalpanek, thanks for the solution here. Also works for me. In my case, I was following the latest recommended way of routing to components using the component property on the state object, and my initial impulse was to try attribute binding on the ui-view directive, which I think may be under consideration by the team, but doesn't currently work.

I also specified an empty url on my parent component (the app's root component) and made the child a redirectTo component. When using the component property instead of a template string, I did not need a url for my root component for the redirect to work (not sure why).

I also tried using bindings in my state definition to alias the parent controller's property, as you can do with resolves, and then referencing the alias in the child component's bindings, but that didn't seem to work.

Now my question is whether this is a hack or should be part of the documentation, because all I saw in the Route To Components docs was the use of the component prop in the state definition. Unless I missed something... This seems to be a much better way to get references to parent properties than using require to get a hold of the entire parent controller.

@christopherthielen
Copy link
Contributor

@michalpanek In RC.1, I've implemented wiring bindings through the ui-view to the child component as has been discussed in this thread.

The Route to Components doc has been updated with details of how to wire through to the children.

my initial impulse was to try attribute binding on the ui-view directive, which I think may be under consideration by the team, but doesn't currently work.

That is now implemented as you describe in rc.1

@pierrekraemer
Copy link
Author

Thanks for the new feature :-)
What I learn w.r.t. my initial problem here is that for a routed component to be informed that its resolved input bindings have been reassigned, its state has to be reloaded so that its bindings get refreshed. Am I right ?
In the docs example, the userDetail input binding is reassigned in the handleUserUpdated function, but the userlist.detail state has to be reloaded in order for the user to be refreshed. In the example, you have to transition to this state because userDetail and userEdit share the same ui-view, but if it was not the case, I still would have expect the $onChanges lifecycle hook of userDetail to be triggered when the handleUserUpdated reassigns the user. This would maybe allow the code of the handleUserUpdated function to be reduced to its minimum which is the reassignement of the user. It kinds of trouble me that this function has to know in which state to transition after the user update. What do you think ?

@pierrekraemer
Copy link
Author

I forked the docs example plunker to illustrate my concern : https://plnkr.co/edit/vrxNroAegA2iNWYtulu3?p=preview
I merged the userDetail and userEdit components (and states). The userDetail component displays the binded user and uses a local copy for edition purpose. This copy is passed up to the users component on update, which reassigns the corresponding user in the handleUserUpdated function. I commented the $state.go instruction here which I find superfluous. But of course, it does not work without it..

@christopherthielen
Copy link
Contributor

The resolve process happens when a state is entered (during a Transition). Once a state has been entered, the resolved data does not change (until it is exited and re-entered, i.e., reloaded).

Therefore, changes to resolve data has to occur when a state is exited/entered. Reloading has two further effects:

    1. it invalidates all child resolves
    1. it reloads the component tree

There is no way for $onChanges() to respond to resolve data changes (after a state reload) because the component tree is reloaded. There is no way to modify resolve data, because it is a one-time process. I hope this helps explain why it behaves the way it does.


I will respond to some comments:

In the docs example, the userDetail input binding is reassigned in the handleUserUpdated function

I still would have expect the $onChanges lifecycle hook of userDetail to be triggered when the handleUserUpdated reassigns the user.

My response to both these is: no, the data in the parent component's local variable was updated. We never updated the variable the userDetail input is bound to. The userDetail input binding is in fact bound to $resolve.user. (you can see this detail by inspecting the DOM tree)

It kinds of trouble me that this function has to know in which state to transition after the user update. What do you think ?

Something has to know what to do when the user is updated. In this example, users is the "smart component" which knows what the proper action is. Seems OK to me. Of course, users could be converted to a dumb component too, but then something else must have the smarts of what to do when a user is updated. In the example, I gave that responsibility to the users component. Where do you think that logic belongs?

I merged the userDetail and userEdit components (and states)

Thats also a good design choice, but is simply personal preference. I chose to make editing of a user into a separate state with corresponding URL. The rest of the behavior will be essentially the same as in the 2 state approach.


The router will update data with the states change. It does not attempt to keep invoking resolve functions over time, unless the state changes.

If you don't want to use the router to manage your application state, you can certainly manage it manually, or use other state management tools such as redux or ngrx/store.

Here's an example forked from yours that manually manages the state of the child component state: https://plnkr.co/edit/omsyOD0GJSh0cm3JCE40?p=preview

@christopherthielen
Copy link
Contributor

Note that because I'm refuting some of your opinions does not mean that there can be no changes to how it's done today. Only that this is how it is done today, and why... with some potential alternatives.

Please keep the discussion going 👍

@pierrekraemer
Copy link
Author

pierrekraemer commented Jan 12, 2017

Thank you very much for your answer and insights.
Actually I think it would seem more natural to me in the original example if the transition from the userlist.detail.edit to the userlist.detail state was done by the userEdit component when clicking on the "Update user" button because this is the place where the action that drives the interaction takes place. The handleUserUpdated function of the userList component could be called from somewhere else and should not have this responsibility.
Anyway, keep the good work going 👍

@orneryd
Copy link

orneryd commented Feb 17, 2017

I don't know for sure if this is related. However, what I want to be able to do, and haven't found a "clean" way to do, is reload the bindings/resolve on a state without a re-render (destroy/create) of the component being routed to.

This can be especially useful for components which render things like maps or large stateful 3rd party UI components that are costly to re-render just because a parameter changed. Managing that state in a higher-order component works but is not ideal.

@christopherthielen
Copy link
Contributor

@timothyswt before route to component, updating of resolves was a non starter, because resolves had to be injected. Now that they can be bound to a component, I'm more open to alternatives that update the bindings dynamically.

It's also worth noting that observables work great in the current paradigm and still allow the async logic to be outside the component in the resolve block.

@orneryd
Copy link

orneryd commented Feb 17, 2017

Thanks! I thinking would be pretty cool if we could set a flag on a state or on a view to say something like rebindable: true or an option in state.go to say "rebind my resolves but do not reinitialize the component"

@roypeled
Copy link

Any update on this?
This could be really good for stateless components! Instead of re-rendering the entire component, I would love to be notified of bindings change (via $onChanges) and perform an optimized render myself.

@mr-moon
Copy link

mr-moon commented Jun 20, 2018

@christopherthielen just been through your docs at https://ui-router.github.io/guide/ng1/route-to-component#routed-parentchild-component-communication but how would you take care of sibling routed components? Say that userEdit component is routed by userlist.detail.edit state, however, in the dom, it's view is not nested within userList component's view, but rather has own named view somewhere in the <body ui-view>?

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

No branches or pull requests