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

perf(client): share replay of computed observables #1095

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

arnautov-anton
Copy link
Contributor

@arnautov-anton arnautov-anton commented Sep 18, 2023

Overview

Adds shareReplay to computed observables which when previously subscribed to by multiple components would be evaluated separately, i.e., each source data emission would evaluate defined pipeline for each subscription individually.

"Futureproofs" RxUtils.getCurrentValue method through usage of combineLatest function which will always take the last emitted value even from "pure" observables unlike take operator which "resolves" with first emitted value.

Fixes noop sorting (\w) test where "sort" mutates source data - when we move to TS5.2 we should replace that with Array.toSorted.

Implementation notes

I mentioned using share operator which uses refCount and Subject under the hood meaning that if amount of subscriptions on such observable reaches 0 (refCount === 0) - it resets the Subject and re-evaluates the pipeline and assignes new value on next subscription. Using it in our SDK proved to be difficult and was behaving unexpectedly (broke tests and certain observables would return "undefined" instead of initial values) though on paper it makes perfect sense. I opted to use shareReplay which by default (with only buffer size as argument) works like a ReplaySubject but continues running defined pipeline (never unsubscribes from the source observable) even after the refCount reached zero. With the overloaded variant shareReplay({ bufferSize: 1, refCount: true }) it behaves just as expected and does not break our SDK - meaning the values (pipelines) are being computed only once per multiple subscriptions and only if it has at least one subscription.

Sources:

@arnautov-anton arnautov-anton force-pushed the perf/computed-observables-share-replay branch 2 times, most recently from 6e69c38 to c474c58 Compare September 20, 2023 10:38
Copy link
Contributor

@szuperaz szuperaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arnautov-anton I think you will be our new RxJS expert, I've never used the refCount thing before.

@arnautov-anton arnautov-anton force-pushed the perf/computed-observables-share-replay branch from c474c58 to 5d8eda7 Compare September 20, 2023 13:02
@arnautov-anton arnautov-anton force-pushed the perf/computed-observables-share-replay branch from 5d8eda7 to 91d9734 Compare September 22, 2023 11:17
@oliverlaz oliverlaz merged commit 759d9a2 into main Sep 26, 2023
19 checks passed
@oliverlaz oliverlaz deleted the perf/computed-observables-share-replay branch September 26, 2023 14:37
arnautov-anton added a commit that referenced this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants