Skip to content

feat(cache): remove cache operator#2012

Merged
jayphelps merged 1 commit intoReactiveX:masterfrom
benlesh:remove_cache
Oct 7, 2016
Merged

feat(cache): remove cache operator#2012
jayphelps merged 1 commit intoReactiveX:masterfrom
benlesh:remove_cache

Conversation

@benlesh
Copy link
Copy Markdown
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
Copy Markdown
Member Author

benlesh commented Oct 6, 2016

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

@benlesh
Copy link
Copy Markdown
Member Author

benlesh commented Oct 6, 2016

cc/ @kwonoj @staltz @jayphelps @trxcllnt

@benlesh
Copy link
Copy Markdown
Member Author

benlesh commented Oct 6, 2016

more related: #2010 #1959

@coveralls
Copy link
Copy Markdown

Coverage Status

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

@jayphelps
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

trxcllnt commented Oct 7, 2016

LGTM

@staltz
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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