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

Call the changedHandler after the value has been set #69

Open
Kennethtruyers opened this issue Oct 4, 2018 · 5 comments · May be fixed by #82
Open

Call the changedHandler after the value has been set #69

Kennethtruyers opened this issue Oct 4, 2018 · 5 comments · May be fixed by #82
Labels
breaking-change This PR introduces a breaking change enhancement

Comments

@Kennethtruyers
Copy link

I'm submitting a feature request

  • Library Version:
    1.1.0

Please tell us about your environment:

  • Operating System:
    all

  • Node Version:
    all

  • NPM Version:
    all
  • JSPM OR Webpack AND Version
    both
  • Browser:
    all

  • Language:
    all

Current behavior:
Whenever we subscribe to a part of the state, the changed handler will be called before the actual state is changed.

@autoinject()
@connectTo({
  selector: {
    list: store => store.state.pipe(pluck('list'))
  }
})
export class App {
  store;
  list : string[] = [];
  normalBinding : string;

  constructor(store : Store<any>) {
    this.store = store;
    this.store.registerAction('addList', addList);
  }

  listChanged(newValue, oldValue) {
    // this.list does not have the newValue yet, in contrast with normal 
    // aurelia behavior where this.list would have the value when this handler is called
  }

   normalBindingChanged(newValue, oldValue){
     // this.normalBinding is already set at this moment
   }
  }
}
  • What is the expected behavior?
    The bound value should be set before the changed-handler is invoked

  • What is the motivation / use case for changing the behavior?
    It aligns better with the default Aurelia behavior and meets the expectations of users better

@jmzagorski
Copy link
Contributor

jmzagorski commented Oct 4, 2018

@zewa666 I do not see a problem with this change as I hardly ever use the newVal argument with Aurelia's change handling since the value is already set. I think i left the execution the same as it was prior to the PR because of this comment in the code:

call onChanged first so that the handler has also access to the previous state

However, based on the other thread it seems that you are fine with the change. Hopefully it just involves moving code as the change handers are fired here and property setting is done here

@Vheissu
Copy link
Member

Vheissu commented Oct 26, 2018

This issue is related to the fact that the connectTo decorator attaches subscriptions inside of bind and unbind but I've found sometimes this is too late and you get the problem you speak of. I find if I setup a subscription inside of the constructor itself then I don't run into these issues where the value is unavailable. It's a decent workaround.

Not sure if your provided code was just a non-function example, but your list property is not an observable so Aurelia won't call listChanged because it is not observing the property. Although, you might know this already.

@jmzagorski
Copy link
Contributor

@zewa666 I can work on this since I am familiar with the changes I submitted for the connectTo decorator.

@Vheissu yes i agree, especially when using the aurelia-router because activate fires first and sometimes i need my selectors during the activate method, so I am always overriding the connectTo setup/teardown functions

@rmja
Copy link

rmja commented Jan 13, 2020

Hi all, what is the status of this issue?

@zewa666 zewa666 added the breaking-change This PR introduces a breaking change label Jan 14, 2020
@zewa666
Copy link
Member

zewa666 commented Jan 14, 2020

Hey @rmja. It was left open due to the reason that it's going to introduce a breaking change and the actual value of the change is relatively small. So my thinking was to either wait until there is something else thats going to create one or do this with the port to Aurelia2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This PR introduces a breaking change enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants