Skip to content
This repository was archived by the owner on Oct 12, 2021. It is now read-only.

fix(worker): correctly emit falsy values sent by the ServiceWorker #137

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(worker): correctly emit falsy values sent by the ServiceWorker #137

wants to merge 1 commit into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Mar 29, 2017

Currently, there is check that completes the observable on falsy values (takeWhile(v => !!v)). This was first introduced in ffea4acfe, when there was no other mechanism for completing the observable (and possibly all sent values were objects). Then a different mechanism was introduced in 563e7ac44, with a stricter check (data === null).

Since there are types of messages that send boolean values (e.g. checkForUpdate or activateUpdate), it is necessary to not complete the observable on false.

Currently, there is check that completes the observable on falsy values
(`takeWhile(v => !!v)`). This was first introduced in [ffea4ac][1], when there
was no other mechanism for completing the observable (and possibly all sent
values were objects). Then a different mechanism was introduced in
[563e7ac][2], with a stricter check (`data === null`).

Since there are types of messages that send boolean values (e.g.
[`checkForUpdate`][3] or [`activateUpdate`][4]), it is necessary to not complete
the observable on `false`.

[1]: ffea4ac#diff-a223d9a67ca2c0a46e98a4f481f74944R133
[2]: 563e7ac#diff-a223d9a67ca2c0a46e98a4f481f74944R148
[3]: https://github.com/angular/mobile-toolkit/blob/f9d83026ea75af1f3a2848903e4072bce19cb5e9/service-worker/worker/src/worker/driver.ts#L850
[4]: https://github.com/angular/mobile-toolkit/blob/f9d83026ea75af1f3a2848903e4072bce19cb5e9/service-worker/worker/src/worker/driver.ts#L856
@gkalpak
Copy link
Member Author

gkalpak commented Mar 29, 2017

@alxhub, I tried adding some tests but couldn't find an easy way. Glad to work on it if you can give me some pointers.

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.

1 participant