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

Store in cache observable itself but not it's value #115

Open
1 of 8 tasks
LendaVadym opened this issue Mar 11, 2019 · 2 comments
Open
1 of 8 tasks

Store in cache observable itself but not it's value #115

LendaVadym opened this issue Mar 11, 2019 · 2 comments

Comments

@LendaVadym
Copy link

LendaVadym commented Mar 11, 2019

I'm submitting a ... (check one with "x")

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  <!-- Please check the repository for a similar issue or PR before submitting -->
[x] Support request => <!-- Please check the repository for a similar issue or PR before submitting -->
[x] Feature request
[ ] Documentation issue or request

Current behavior
Currently, if decorated with @cached method returns observable we are waiting for observable to emit data (pipeble method map is used) to save this data in cache. It's not working with cold observables when f.e. several angular services request the same data (http) simultaneously at startup. Each service will receive its own observable and several remote calls will be invoked. The last returned value will be stored in cache.

Expected/desired behavior
Wouldn't it be more correct to store in cache the returned from decorated method observable itself (f.e. as ReplaySubject)? Then every subsequent call to the decorated method will return the same observable and every angular service that invoke decorated method can subscribe tho this observable and get data.

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?
To reduce number of remote calls if decorated with @cached method is called simultaneously several times.

Environment

  • Angular version: X.Y.Z
  • Browser:
  • Chrome (desktop) version XX
  • Chrome (Android) version XX
  • Chrome (iOS) version XX
  • Firefox version XX
  • Safari (desktop) version XX
  • Safari (iOS) version XX
  • IE version XX
  • Edge version XX
  • For Tooling issues:
  • Node version: XX
  • Platform:
  • Others:
@LendaVadym
Copy link
Author

LendaVadym commented Mar 11, 2019

Code from @cached decorator:

How it's currently implemented:
...

const value = method.apply(this, args);
if (isObservable(value)) {
   return value.pipe(
      map((res: any) => {
         cache.set(cacheKey, res, ReturnType.Observable);
            return res;
          })
        );
      }

...

Proposition:
...

if (isObservable(value)) {
   const subject = new ReplaySubject<any>(1);
   value.subscribe(
         (next) => { 
            subject .next(r); 
         },
         (error) => {
            // possibly clear subject from cache;
            subject .error(e);
         },
         () => subject .complete()
      );

   cache.set(cacheKey, subject.asObservable(), ReturnType.Observable);   
}

@fulls1z3
Copy link
Owner

@LendaVadym sorry for the delay, had busy times. feel free to fire a PR and i'll approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants