fix(effects): resubscribe every time an error occurs #2165
Conversation
| // Return observable that produces this particular effect | ||
| return resubscribeInCaseOfError(observable$); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Can you please make it as a package-private function declaration and put it outside of mergeEffects?
| @@ -24,14 +24,19 @@ export function mergeEffects( | |||
| ? sourceInstance[propertyName]() | |||
| : sourceInstance[propertyName]; | |||
|
|
|||
| const resubscribeInCaseOfError = ( | |||
| observable$: Observable<any> | |||
There was a problem hiding this comment.
both any should be replaced with T to make sure that both Observables are of the same kind.
|
Preview docs changes for a6e9ba1 at https://previews.ngrx.io/pr2165-a6e9ba1/ |
| /** | ||
| * @internal | ||
| */ | ||
| export function resubscribeInCaseOfError<T extends Action = Action>( |
There was a problem hiding this comment.
No need to export it, right?
There was a problem hiding this comment.
Right, it also doesn't need the @internal comment
| /** | ||
| * @internal | ||
| */ | ||
| export function resubscribeInCaseOfError<T extends Action = Action>( |
There was a problem hiding this comment.
Is = Action needed? I think just extending Action is fine.
There was a problem hiding this comment.
No, it isn't needed. Thanks for the review 👍
|
Preview docs changes for 5151b3d at https://previews.ngrx.io/pr2165-5151b3d/ |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Seems like with the original PR we forgot to modify the spec for the usage with
createEffect. This PR uses the same implementation as the original PR.History: