WIP Rxjs 6 and angular 6

[kawamoto]

I've try to update rxjs to v6.x #1538

It pass a build, and seems work fine with my tiny app which is using authentication and realtime db.
Test is failed and I can not figure out why. it seems injection in test does not working as expected.

I hope it helps maintainers or someone else.

Checklist

• Issue number for this PR: #nnn (required)
  • RXJS 6 compatibility #1538
• Docs included?: (yes/no; required for all API/functional changes)
  • no
• Test units included?: (yes/no; required)
  • no
• In a clean directory, yarn install, yarn test run successfully? (yes/no; required)
  • no

[davideast]

Hi @kawamoto! I really appreciate you taking the time to migrate us over to RxJS 6. However, I do not recommend doing it all in one big PR. It's really hard to review and increases the chances of creating bugs. How about you partition this PR into multiple ones based off each Firebase feature? Start with changing one module and after we merge that in move on to the next. Does that sound good?

[ghost]

@kawamoto Do you mind if I take this over or are you working on the changes?

[kawamoto]

Hi @davideast. thank you for respond. I basically agree with your suggestion. but with this topic, I think it's hard to split PR simply. without whole of those changes, even building does not work. maybe it's better to update latest RxJS 5.x and migrate operators first. after that, I guess updating process looks more simple. wdyt?

[kawamoto]

Hi @robertbaker , I'm happy with someone solve this 👍 I guess I can not take time for this PR in this week.

[ghost]

@davideast I would prefer to do Rx6 has 1 clean PR, I see risk mixing Rx5 and 6 together. One PR also allows a beta release for people to test. I'm happy to work on this and chat if needed. Let me know your thoughts.

[juanpablodelatorre]

Is there any new information about this? Can we expect this to be working any time soon?

[davideast]

@kawamoto @robertbaker I appreciate your efforts, but this is a big change that has to be carefully done. This PR already has 48 changed files this is too large to review without missing any changes. At Google we measure the size of CLs (our version of PRs) and this would be an XL CL. It is strongly recommend to keep your CLs at small to medium so they are easier to review. XL CLs do happen, but it is advisable to avoid them.

You can continue to upgrade the pipeable operators per feature using RxJS 5.5+. That means there would be one PR per feature to migrate to pipeable operators. When we're off the side effect import operators we can then do a PR to upgrade to RxJS 6 and fix and changed import paths.

Calling in @benlesh to see if this is a sound strategy.

[jymdman]

@davideast I have made a couple of pull request to update this lib to pipeable operators. I hope this is what you are looking for to get this thing going! :)
(left out database-deprecated as this will be dropped in another pr...)

[jamesdaniels]

Closing in favor of #1625

[davideast]

@robertbaker @jymdman @kawamoto We've had a lot of interest in creating PRs to upgrade to RxJS. We need to be clear about who is making these changes and in what manner. We also need a fairly fast timeline. As seen earlier @jamesdaniels is working fervently to get AngularFire up to speed with the latest changes in Angular, RxJS, and Firebase.

We would love contributions from the community on upgrading to RxJS 6. This upgrade should be done by creating one PR per Firebase feature to migrate to pipeable operators with RxJS 5.5. When we're off the side effect import operators we can then do a PR to upgrade to RxJS 6 and fix and changed import paths. Let me know who is interested and we will hold the set of PRs for you.

If you prefer @jamesdaniels implements it quickly then let us know. Sorry about the lack of communication behind this issue. In the future when sending a PR for an issue, please comment asking to work on it. This way we can all stay in sync on who is working on what.

[davideast]

Update: I noticed that @jamesdaniels pulled the PRs in from @jymdman, which were nice and small! I needed my coffee before trying to get up to speed with the latest PRs.