Rxjs interop updates #49769
Closed
Rxjs interop updates #49769
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atscott
added
area: core
|
Are these changes too late for the RC ? |
jessicajaniuk
approved these changes
Apr 11, 2023
These kinds of changes are exactly the type of thing we should change in the RC period (also RC isn't until tomorrow). Based on feedback during this period, we can and should update features that will go into the final release. |
atscott
added
target: rc
dylhunn
reviewed
Apr 11, 2023
| // fromObservable(Observable<Animal>) -> Signal<Cat> | ||
| source: Observable<T>, options: {requireSync: true}): Signal<T>; | ||
| export function fromObservable<T, U = undefined>( | ||
| source: Observable<T>, options?: {initialValue?: U, requireSync?: boolean}): Signal<T|U> { |
There was a problem hiding this comment.
Nit: I'd add an explanatory comment that this signature has to be listed second
There was a problem hiding this comment.
It doesn't though, does it? The first two can be in either order. Or do you mean the last one has to be listed third?
dylhunn
approved these changes
Apr 11, 2023
atscott
force-pushed
the
rxjsInteropUpdates
branch
3 times, most recently
from
4a777ae
to
fd42d6a
Compare
The initial value used for signals by default is now `undefined`. In addition, there is a new option to express that the signal should emit a value synchronously (`requireSync: true`). When this value is specified, the function will throw _on creation_ if the subscribing to the `Observable` does not result in a synchronous emit.
Based on feedback in the RFC, most would prefer `toSignal` and `toObservable`.
From Ben: > When dealing with any reactive function call you don't control > like `observer.next()` (or anything similar), you want to catch the error > in the producer call, in this case `signal()`. You don't want to catch errors > in the `observer.next` call itself.
`toObservable` creates an `effect` that watches for updates to the source signal. We should allow writes to signals in this effect, which would be consumed by downstream observers.
atscott
force-pushed
the
rxjsInteropUpdates
branch
from
fd42d6a
to
ec9d734
Compare
MaximSagan
reviewed
Apr 12, 2023
| * | ||
| * @developerPreview | ||
| */ | ||
| export interface FromSignalOptions { | ||
| export interface toObservableOptions { |
|
Caretaker note: adding the "merge" label based on the conversation with @atscott. |
|
This PR was merged into the repository by commit 1dddb78. |
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 12, 2023
#49769) The initial value used for signals by default is now `undefined`. In addition, there is a new option to express that the signal should emit a value synchronously (`requireSync: true`). When this value is specified, the function will throw _on creation_ if the subscribing to the `Observable` does not result in a synchronous emit. PR Close #49769
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 12, 2023
From Ben: > When dealing with any reactive function call you don't control > like `observer.next()` (or anything similar), you want to catch the error > in the producer call, in this case `signal()`. You don't want to catch errors > in the `observer.next` call itself. PR Close #49769
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 12, 2023
From Ben: > When dealing with any reactive function call you don't control > like `observer.next()` (or anything similar), you want to catch the error > in the producer call, in this case `signal()`. You don't want to catch errors > in the `observer.next` call itself. PR Close #49769
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits