Reactive-Extensions / rx.angular.js

AngularJS Bindings for RxJS
Other
827 stars 89 forks source link

Update rx.angular.js to use/work with RxJS 5 beta 8 #146

Open awk opened 8 years ago

awk commented 8 years ago

This updates the library and tests to pass with RxJS 5 beta 8, the changes are a mixture of 'cosmetic' (method renames) and slightly more structural (using Rx.Subscription).

With these changes the unit tests now pass - NOTE: I've not (yet) updated the examples for RxJS 5

paulpdaniels commented 8 years ago

IMO this should wait until RxJS5 comes out of beta. Rx.angular.js is in a stable version as is the underlying RxJS4, I think we should expect the same from RxJS5 before incorporating it.

awk commented 8 years ago

I actually agree with you - one of the things with this code is that I did enough to make the javascript 'work'.

There's an additional set of changes I've been working on which changes things further so that the rx-angular code is written in typescript and can incorporate the type information etc. from rxjs5.

I will probably update my forked repo (the source branch for this PR) in the coming weeks with those typescript changes - there's a bunch of work still to be done around building and packaging the code though.

johannesjo commented 7 years ago

Any news on this one?

mattbrunetti commented 7 years ago

@paulpdaniels It seems like it's ready now. It's well past beta, and currently at v5.3.0

Does anyone know if there are more breaking changes in the rxjs API (since beta 8) that will need to be integrated into this?

Is anyone using this branch?

@sirbarrence Is the PR/branch likely to be merged in the end if it also includes changes to move from ES5 to TypeScript? I would not want to have problems of that sort.

kylecordes commented 7 years ago

I had forgotten this adapter exists, and set about writing some code using RxJS 5 with AngularJS 1.6. Happily, I never found a need for observeOnScope($scope, .. or $scope.$createObservableFunction(.... You can see / run the results:

https://github.com/OasisDigital/angularjs-rxjs-example

Or watch a video explanation:

https://www.youtube.com/watch?v=XY9VPrKuJaA

In this example, I never injected or used $scope in the application code (scope is used behind the scenes in a library that bring something very close to the Angular 2+ async pipe in as a filter: https://github.com/cvuorinen/angular1-async-filter ). Application references to $scope, as well as $scope.watch, are now discouraged in new AngularJS code. Also, people doing heavily reactive work tend to minimize use of .subscribe().

Keeping all that in mind, I think maybe the rx.angular.js features are most useful for upgrading existing older AngularJS code that uses RxJS 4 to RxJS 5? Vs. new code?

mattbrunetti commented 7 years ago

@kylecordes Thank you for the insight. I'll be using RxJS 5 directly in our project, and give an update if I find any general need for the functionality here.

sirbarrence commented 7 years ago

@mattbrunetti Probably a question for @mattpodwysocki

I would like to see rx.angular officially support RxJS 5 though. I haven't had much luck migrating my RxJS 4 / Angular 1.5 projects to RxJS 5 yet, partially due to my own failed attempts to get rx.angular working with it and due to having to reimplement operators missing from RxJS 5.

paulpdaniels commented 7 years ago

We have a couple projects currently using rxjs 5 with angular 1.6, so far haven't had much need for the adapter. I would like to see this get merged in at some point as I think there are some good parts that would encourage more reactive practices amongst my team.

@awk are you still interested in shepherding this along?

awk commented 7 years ago

This pull request is now very old, it almost certainly needs a bit of an overhaul from when I first wrote it. I'm happy to do that - but - does anyone who's commented thus far actually have write access to this repo to commit the pull request?

Or - are you suggesting that I should take ownership of the project in its entirety if the current project/repo owner no longer has the time or resources to continue things?

paulpdaniels commented 7 years ago

@awk I don't have merge rights, AFAIK that is only @mattpodwysocki . I meant just this PR not necessarily the repo.

mattbrunetti commented 7 years ago

@paulpdaniels Good to know that although there isn't so much a need for this package with "new angularjs code" (i.e. angularjs 1.6 and rxjs 5) there are still some parts that could be helpful. I'll keep an eye on the documentation here as I prototype using rxjs directly.

Once rxjs 5 support is added, It would be great to add a bit of best-practices knowledge to the documentation, noting which parts would be discouraged in new angularjs projects (e.g. observeOnScope) vs which we (angular 1.6 developers) should take note of

mattbrunetti commented 7 years ago

Rx.Observable#safeApply seems to a required bit of functionality, to ensure a digest cycle happens when updates on the $scope are triggered by stream events that are not triggered by $http, dom events, etc.

aabm00 commented 7 years ago

Hi

Is there some news when the version of rx.angular.js with support for RxJS 5 will be released?

Thanks