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

fix(pipe): isObservable usage #1411

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

iamnaumovnikita
Copy link

isObservable(res) instead of isObservable(res.subscribe)

I changed the usage of isObservable function to the correct one. Previously, it checks the subscribe function itself while it should check the observable itself. I also add a test that describes in which case it could lead to problem.

Without my fix the test is red and the actual result is

"{"operator": [Function anonymous], "source": {"_subscribe": [Function anonymous]}}"

image

With my fix the test is green.

image

In my test I described the scenario of a race condition when there is a bit delay of loading new language translation (after switching) and the usage MissingHandler with observable takes place.

P.S.

In many cases the next tick will solve the problem and user will see the correct translation, but there is a chance to see the stringified observable "{"operator": [Function anonymous], "source": {"_subscribe": [Function anonymous]}}"

In my PR I also removed some redundat imports.

iamnaumovnikita and others added 2 commits February 8, 2023 00:06
check observable itself instead of subscribe function
@iamnaumovnikita iamnaumovnikita changed the title This PR fixes the misusing of isObservable function in translate.pipe fix(pipe): isObservable usage Feb 9, 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.

None yet

1 participant