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

feat(cache): remove cache operator #2012

Merged
merged 1 commit into from
Oct 7, 2016
Merged

feat(cache): remove cache operator #2012

merged 1 commit into from
Oct 7, 2016

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Oct 6, 2016

Because of all of the confusion around the cache operator, this removes the operator
from RxJS ahead of the 5.0.0 release. This is done for a few reasons:

  1. The name cache implies it supports all sorts of caching strategies, when this is pretty limited
  2. Behavior around unending and hot observables is hard to reason about
  3. Once we have a properly designed cache operator we can add it at a later time and it will not be a breaking change

@benlesh
Copy link
Member Author

benlesh commented Oct 6, 2016

related #839 #1718 #2006 #1417 #1405 and many more I'm sure.

@benlesh
Copy link
Member Author

benlesh commented Oct 6, 2016

cc/ @kwonoj @staltz @jayphelps @trxcllnt

@benlesh
Copy link
Member Author

benlesh commented Oct 6, 2016

more related: #2010 #1959

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 97.051% when pulling 8b347ce on blesh:remove_cache into 5448f87 on ReactiveX:master.

@jayphelps
Copy link
Member

I'm very very 👍 , but since there's a chance this is controversial, going to wait for others to chime in.

@kwonoj
Copy link
Member

kwonoj commented Oct 6, 2016

I'm 👍 too. If it's debatable about behavior need to be settled down, better to make it once instead of changing behavior over different release which can cause confusion.

@trxcllnt
Copy link
Member

trxcllnt commented Oct 7, 2016

LGTM

@staltz
Copy link
Member

staltz commented Oct 7, 2016

LGTM

"Cache invalidation" :trollface:

Because of all of the confusion around the `cache` operator, this removes the operator
from RxJS head of the 5.0.0 release. This is done for a few reasons\:

1. The name `cache` implies it supports all sorts of caching strategies, when this is pretty limited
2. Behavior around unending and hot observables is hard to reason about
3. Once we have a properly designed `cache` operator we can add it at a later time and it will not be a breaking change
@coveralls
Copy link

coveralls commented Oct 7, 2016

Coverage Status

Coverage decreased (-0.01%) to 97.051% when pulling 8b347ce on blesh:remove_cache into 5448f87 on ReactiveX:master.

@jayphelps jayphelps merged commit 0e01d9a into ReactiveX:master Oct 7, 2016
matthewwithanm added a commit to matthewwithanm/flow-typed that referenced this pull request Oct 28, 2016
The `cache()` operator was removed by ReactiveX/rxjs#2012. Since this
is still a prerelease (and Flow doesn't yet support prerelease
versions), we'll just remove it.
jeffmo pushed a commit to flow-typed/flow-typed that referenced this pull request Oct 28, 2016
The `cache()` operator was removed by ReactiveX/rxjs#2012. Since this
is still a prerelease (and Flow doesn't yet support prerelease
versions), we'll just remove it.
igorrafael added a commit to igorrafael/omnisharp-node-client that referenced this pull request Oct 29, 2016
This method was removed from RxJS-5.0.0 release (ReactiveX/rxjs#2012).

Maybe some caching alternative could be implemented later.
For now, it needs to be removed to make the library usable again
(OmniSharp/omnisharp-atom#880).
@colltoaction
Copy link

Hey guys. I can't seem to find a simple alternative to .cache(). I asked this on SO, but maybe you can help me out too.

Thanks!

@benlesh benlesh deleted the remove_cache branch December 16, 2016 03:16
@huan
Copy link
Contributor

huan commented Feb 2, 2017

It takes me some time to finger out that the cache method has gone in v5.0.

I think the better way to remove cache is to show a DEPRECATED message when cache method is called, and keep it for compatible from earlier code bases.

@jayphelps
Copy link
Member

@zixia sorry about the confusion. We made the change back in October when v5 was in beta, where we do not make any guarantees about API stability and deprecation cycles. Some of us have talked about possibly doing deprecation warnings if we do similar sweeping removals for future major versions e.g. v6, v7, etc. but we haven't commited to anything cause we don't yet have any such changes. The only breaking change we have queued up is #2174 which slightly changes the behavior of the buffer operator--it's a bugfix, but since it's a breaking bugfix we're holding off on releasing it until v6.

benlesh pushed a commit that referenced this pull request Jan 11, 2018
@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants