search

neilff / redux-ui-router

ngRedux bindings for Angular UI Router
MIT License
143 stars 48 forks source link

Added stateChangeFinish action to ensure state changes are set in the… #78

Closed hally9k closed 7 years ago

hally9k commented 7 years ago

… app state before components are instantiated.

@Nighthawk14 Do you want to have a look at this for me... I had very minimal time so would be cool to get some verification.

Should fix #74 & #76

Nighthawk14 commented 7 years ago

Looks good @hally9k. Nice one! 👍 . I have tested it on my app and it's seems to behave perfectly.

Although are you sure the PR is meant to be merged with master ? As it will break compatibility with ui-router 0.x and 1.x is still in beta :).

hally9k commented 7 years ago

Thats a really good point actually. Master is currently the version that we will publish as 0.7.0 when ui-router goes 1.0. But I take your point. Maybe master should represent our current stable release? I should start a release-0.7.0 branch and reset master to the previous release version. Then once ui-router 1.0s we can release 0.7.0 and merge into master. Does that sound like the right approach to you @Nighthawk14?

xlozinguez commented 7 years ago

@hally9k did you guys deploy this on npm? I can't seem to fetch it...

npm ERR! No compatible version found: redux-ui-router@^0.7.0
npm ERR! Valid install targets:
npm ERR! 0.6.0, 0.5.2, 0.5.1, 0.5.0, 0.4.4, 0.4.3, 0.4.2, 0.4.0, 0.3.3, 0.3.1, 0.3.0, 0.2.1, 0.2.0, 0.1.4, 0.1.3
xlozinguez commented 7 years ago

I guess I need to use https://www.npmjs.com/package/hally9k-redux-ui-router instead of https://www.npmjs.com/package/redux-ui-router @hally9k ?

Nighthawk14 commented 7 years ago

@xlozinguez Not yet, that will be done as part of the merging of #80. That will probably be not 0.7.0 but 0.7.0-beta.1 anyway :).

hally9k commented 7 years ago

I will get try and deploy the beta version shortly. Sorry for the hold up!

jesperzach commented 7 years ago

@hally9k any updates? :)