Yoni Argivi's snippet for rxjs post

getHttpDistinctResponse.ts

  public getHttpDistinctResponse(httpRequest$: Observable<any>): Observable<any> {
    let cachedResponseStr = null;
    return httpRequest$.pipe(
      filter((response) => {
        const currentResponseStr = JSON.stringify(response);
        const areEquals = cachedResponseStr === currentResponseStr;
        if (!areEquals) {
          cachedResponseStr = currentResponseStr;
        }
        return !areEquals;
      })
    );
  }

export function pollingOnResolved(httpRequest$: Observable<any>, delayMs = 0): Observable<any> {
  const polling$ = new BehaviorSubject({});
  const rePolling$ = 
    of('').pipe(
    delay(delayMs),
    tap(() => polling$.next({})),
    skip(1)
  );
  // BOOM
  const request = ignoreEqualResponses ? this.getHttpDistinctResponse(httpRequest$) : httpRequest$;
  const httpPolling$ = concat(request, rePolling$);
  return polling$.pipe(switchMap(() => httpPolling$));
}
     

how the let cachedResponseStr = null; in line 2 supposed to cache the response? it's a local variable in a local function. it's always null.

Thanks for commenting, that's actually a great question,
I'm not the original author of this code but from few min of wondering about it again (I was the technical reviewer of those snippets) I see that the intention of pollingOnResolved() is to be invoked once, on ngOnInit hook or on such lifetime that stays up in memory (if not in Angular) so basically same for getHttpDistinctResponse() it will be invoked once, but the "poller" does next every X seconds meaning that "request" which is also observable (we should've keep the $ convention..) will eventually be piped so the "cachedResponseStr" is actually memoized, it won't be null every time but I agree this one is confusing..
@yoni12ab can you approve I'm not misleading here?

Yes it is actually a closure

a closure it's a variable outside of a function that is used inside a function. this is not a closure. to make it closure it should be change to this

let cachedResponseStr = null;

  public getHttpDistinctResponse(httpRequest$: Observable<any>): Observable<any> {
    
    return httpRequest$.pipe(
      filter((response) => {
        const currentResponseStr = JSON.stringify(response);
        const areEquals = cachedResponseStr === currentResponseStr;
        if (!areEquals) {
          cachedResponseStr = currentResponseStr;
        }
        return !areEquals;
      })
    );
  }

export function pollingOnResolved(httpRequest$: Observable<any>, delayMs = 0): Observable<any> {
  const polling$ = new BehaviorSubject({});
  const rePolling$ = 
    of('').pipe(
    delay(delayMs),
    tap(() => polling$.next({})),
    skip(1)
  );
  // BOOM
  const request = ignoreEqualResponses ? this.getHttpDistinctResponse(httpRequest$) : httpRequest$;
  const httpPolling$ = concat(request, rePolling$);
  return polling$.pipe(switchMap(() => httpPolling$));
}
     

though technically you have two piece of code that are not homogenous, public getHttpDistinctResponse means you copy it from a class , but export function pollingOnResolved means you copy it from a js module, these two are different and they doesn't work together like this, overall this snippet is incorrect

1. your tone in replying is disrespectful and and I kindly advice you in future to change your approach, honesty you're not impressing anyone by writing "overall this snippet is incorrect" cause js modules which holding utility functions consumed inside methods (of a class) is fine and not a bad practice by any manner, this doesn't make the technique incorrect.
   2.The definition of a closure is as follows:

A closure is the combination of a function bundled together (enclosed) with references to its surrounding state (the lexical environment). In other words, a closure gives you access to an outer function’s scope from an inner function

And bottom line the code fits to that definition in a way that there's an outer variable which is being used in an inner function..

I just stated the truth, the code has obvious bugs and it's incorrect, and it is a random copy-paste from different places. if you believe that "overall snippet is correct" please copy it inside a project and make it works.

I understand you felt attack from my tone because you are the reviewer and you did couple of obvious mistakes, but I don't care who did the mistake, so don't get it personally.

your definition of the closure is correct but your understanding & usage is not. and yet my defintion is correct

a closure it's a variable outside of a function that is used inside a function

that's how shared scope is.

Scope of a function is limited to that function. When two functions are side by side of each other, their shared scope is the file/class. your usage of let cachedResponseStr = null; is incorrect.

moreover, as I stated before because it's not a homogenous snippet, the whole getHttpDistinctResponse() is redundant, a simple distinctUntilChange function at the end of the retrun would do the same since the cache object isn't a real cache and still it's calling the backend on every interval.

Let's agree to disagree.. I'm tired of this thread as it's pointless, that's ok to disagree, you think that my understanding is wrong I think that your understanding is wrong that's fine as long as we're chatting politely, IMO the software community should feel like a "safe" place..

Have a nice weekend and may the code be with you.

disagree on something that is testable, and easy to confirm is ignorance, anyway...
You said the best; you have a good weekend too. ✌️