fix(core): toSignal accepts Subscribable (in addition to Observable) #50162

divdavem · 2023-05-05T12:41:19Z

This PR extends toSignal to accept any Subscribable (instead of only Observable) for consistency with AsyncPipe and for broader compatibility with any observable library (that is compatible with the Subscribable interface).

This is only a type change as the implementation does not use anything else than the Subscribable interface anyway.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

toSignal only accepts Observable.

What is the new behavior?

toSignal accepts any Subscribable.

Does this PR introduce a breaking change?

Yes
No

Other information

This PR is similar to PR 39627 that I had opened for AsyncPipe.

JeanMeche · 2023-05-05T12:49:54Z

packages/core/rxjs-interop/src/to_signal.ts

Observable implements Subscribable, is there a reason you're keeping Observable in the signature ?

You're right, but I saw this code in PipeAsync:

// NOTE(@benlesh): Because Observable has deprecated a few call patterns for `subscribe`, // TypeScript has a hard time matching Observable to Subscribable, for more information // see https://github.com/microsoft/TypeScript/issues/43643 transform<T>(obj: Observable<T>|Subscribable<T>|Promise<T>): T|null;

It looks like Typescript has a bug sometimes.

alxhub

Reviewed-for: public-api

alxhub · 2023-06-09T20:50:20Z

@divdavem thanks for the contribution. Can you change the commit description to label this a fix(core) instead of a feat - this is really fixing a typing issue with toSignal, not introducing new functionality.

divdavem · 2023-06-12T08:44:09Z

@alxhub Thank you for reviewing my PR. I have changed the commit description according to your comment.

Extend toSignal to accept any Subscribable (instead of only Observable) for consistency with AsyncPipe and for broader compatibility with any observable library (that is compatible with the Subscribable interface). This is only a type change as the implementation does not use anything else than the Subscribable interface anyway.

alxhub

Reviewed-for: public-api,fw-core

AndrewKushnir

Reviewed-for: public-api

pkozlowski-opensource

LGTM

Reviewed-for: public-api
Reviewed-for: fw-core

pkozlowski-opensource · 2023-06-14T08:50:02Z

This PR was merged into the repository by commit 1ad4d55.

pkozlowski-opensource · 2023-06-14T08:50:20Z

Thnx @divdavem !!!

Extend toSignal to accept any Subscribable (instead of only Observable) for consistency with AsyncPipe and for broader compatibility with any observable library (that is compatible with the Subscribable interface). This is only a type change as the implementation does not use anything else than the Subscribable interface anyway. PR Close #50162

divdavem · 2023-06-14T09:09:10Z

@alxhub @AndrewKushnir @pkozlowski-opensource Thank you for reviewing and merging my PR!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/16.0.4/16.1.1) | | [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/16.0.4/16.1.1) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/16.0.4/16.1.1) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/16.0.4/16.1.1) | | [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/16.0.4/16.1.1) | | [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/16.0.4/16.1.1) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/16.0.4/16.1.1) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/16.0.4/16.1.1) | | [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.13.0` -> `0.13.1`](https://renovatebot.com/diffs/npm/zone.js/0.13.0/0.13.1) | --- ### Release Notes <details> <summary>angular/angular (@angular/animations)</summary> ### [`v16.1.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1611-2023-06-14) [Compare Source](angular/angular@16.1.0...16.1.1) ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [71360b3a3e](angular/angular@71360b3) | fix | libraries compiled with v16.1+ breaking with Angular framework v16.0.x ([#50715](angular/angular#50715)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [d9bed48eb5](angular/angular@d9bed48) | fix | extend toSignal to accept any Subscribable ([#50162](angular/angular#50162)) | ##### migrations | Commit | Type | Description | | -- | -- | -- | | [5e1d8444ae](angular/angular@5e1d844) | fix | Prevent a component from importing itself. ([#50554](angular/angular#50554)) |  ### [`v16.1.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1610-2023-06-13) [Compare Source](angular/angular@16.0.6...16.1.0) ##### compiler | Commit | Type | Description | | -- | -- | -- | | [4e663297c5](angular/angular@4e66329) | fix | error when reading compiled input transforms metadata in JIT mode ([#50600](angular/angular#50600)) | | [721bc72649](angular/angular@721bc72) | fix | resolve deprecation warning with TypeScript 5.1 ([#50460](angular/angular#50460)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [68017d4e75](angular/angular@68017d4) | feat | add ability to transform input values ([#50420](angular/angular#50420)) | | [69dadd2502](angular/angular@69dadd2) | feat | support TypeScript 5.1 ([#50156](angular/angular#50156)) | | [c0ebe34cbd](angular/angular@c0ebe34) | fix | add additional component metadata to component ID generation ([#50336](angular/angular#50336)) | ##### http | Commit | Type | Description | | -- | -- | -- | | [85c5427582](angular/angular@85c5427) | feat | Introduction of the `fetch` Backend for the `HttpClient` ([#50247](angular/angular#50247)) |  ### [`v16.0.6`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1606-2023-06-13) [Compare Source](angular/angular@16.0.5...16.0.6) ##### core | Commit | Type | Description | | -- | -- | -- | | [05ac0868c9](angular/angular@05ac086) | fix | avoid duplicated content during hydration while processing a component with i18n ([#50644](angular/angular#50644)) |  ### [`v16.0.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1605-2023-06-08) [Compare Source](angular/angular@16.0.4...16.0.5) ##### compiler | Commit | Type | Description | | -- | -- | -- | | [703b8fcac1](angular/angular@703b8fc) | fix | do not remove comments in component styles ([#50346](angular/angular#50346)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [2b6da93e19](angular/angular@2b6da93) | fix | incorrectly throwing error for self-referencing component ([#50559](angular/angular#50559)) | | [c992109d6c](angular/angular@c992109) | fix | wait for HTTP in `ngOnInit` correctly before server render ([#50573](angular/angular#50573)) | ##### platform-server | Commit | Type | Description | | -- | -- | -- | | [c0d4086c6e](angular/angular@c0d4086) | fix | surface errors during rendering ([#50587](angular/angular#50587)) |  </details> <details> <summary>angular/angular (zone.js)</summary> ### [`v0.13.1`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#v0131-httpsgithubcomangularangularcomparezonejs-0130zonejs-v0131-2023-06-09) [Compare Source](angular/angular@zone.js-0.13.0...zone.js-0.13.1) ##### Bug Fixes - **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#50467](angular/angular#50467)) ([381cb98](angular/angular@381cb98)) - **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#50530](angular/angular#50530)) ([7837f71](angular/angular@7837f71)) - **zone.js:** patch entire promise in node ([#50552](angular/angular#50552)) ([cb31dbc](angular/angular@cb31dbc)), closes [#50513](angular/angular#50513) [#50457](angular/angular#50457) [#50414](angular/angular#50414) [#49930](angular/angular#49930) - **zone.js:** revert Mocha it.skip, describe.skip method patch ([#49329](angular/angular#49329)) ([5a2b622](angular/angular@5a2b622)) ##### Features - **zone.js:** jest 29 should ignore uncaught error console log ([#49325](angular/angular#49325)) ([bc412fd](angular/angular@bc412fd)), closes [#49110](angular/angular#49110) ##### Reverts - Revert "fix(zone.js): enable monkey patching of the `queueMicrotask()` API in node.js ([#50467](angular/angular#50467))" ([#50529](angular/angular#50529)) ([7567348](angular/angular@7567348)), closes [#50467](angular/angular#50467) [#50529](angular/angular#50529) [#50529](angular/angular#50529) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1927 Reviewed-by: Epsilon_02 <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>

AndrewKushnir · 2023-06-17T17:35:09Z

@divdavem thanks for the contribution 👍

angular-automatic-lock-bot · 2023-07-18T00:13:15Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

Extend toSignal to accept any Subscribable (instead of only Observable) for consistency with AsyncPipe and for broader compatibility with any observable library (that is compatible with the Subscribable interface). This is only a type change as the implementation does not use anything else than the Subscribable interface anyway. PR Close angular#50162

angular-robot bot added the detected: feature PR contains a feature commit label May 5, 2023

JeanMeche reviewed May 5, 2023

View reviewed changes

divdavem force-pushed the toSignalForSubscribable branch 2 times, most recently from 12d4903 to 253c07a Compare May 5, 2023 14:08

divdavem changed the title ~~[WIP] feat: toSignal accepts Subscribable (in addition to Observable)~~ feat: toSignal accepts Subscribable (in addition to Observable) May 5, 2023

divdavem marked this pull request as ready for review May 5, 2023 14:25

pullapprove bot requested a review from alxhub May 5, 2023 14:25

AndrewKushnir added area: core Issues related to the framework runtime cross-cutting: signals labels May 17, 2023

ngbot bot added this to the Backlog milestone May 17, 2023

alxhub approved these changes Jun 9, 2023

View reviewed changes

pullapprove bot requested a review from alxhub June 9, 2023 20:49

divdavem force-pushed the toSignalForSubscribable branch from 253c07a to 209091e Compare June 12, 2023 08:40

angular-robot bot removed the detected: feature PR contains a feature commit label Jun 12, 2023

divdavem changed the title ~~feat: toSignal accepts Subscribable (in addition to Observable)~~ fix(core): toSignal accepts Subscribable (in addition to Observable) Jun 12, 2023

divdavem force-pushed the toSignalForSubscribable branch from 209091e to 8b6b190 Compare June 12, 2023 08:45

alxhub approved these changes Jun 12, 2023

View reviewed changes

pullapprove bot requested review from AndrewKushnir, dylhunn and pkozlowski-opensource June 12, 2023 23:02

AndrewKushnir approved these changes Jun 12, 2023

View reviewed changes

pkozlowski-opensource added the target: patch This PR is targeted for the next patch release label Jun 14, 2023

pkozlowski-opensource approved these changes Jun 14, 2023

View reviewed changes

pkozlowski-opensource removed the request for review from dylhunn June 14, 2023 08:43

pkozlowski-opensource added the action: merge The PR is ready for merge by the caretaker label Jun 14, 2023

pkozlowski-opensource closed this in 1ad4d55 Jun 14, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 18, 2023

fix(core): toSignal accepts Subscribable (in addition to Observable) #50162

fix(core): toSignal accepts Subscribable (in addition to Observable) #50162

Conversation

divdavem commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

JeanMeche May 5, 2023

Choose a reason for hiding this comment

Uh oh!

divdavem May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

alxhub commented Jun 9, 2023

Uh oh!

divdavem commented Jun 12, 2023

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

Uh oh!

pkozlowski-opensource commented Jun 14, 2023

Uh oh!

pkozlowski-opensource commented Jun 14, 2023

Uh oh!

divdavem commented Jun 14, 2023

Uh oh!

AndrewKushnir commented Jun 17, 2023

Uh oh!

angular-automatic-lock-bot bot commented Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

divdavem commented May 5, 2023 •

edited

Loading

divdavem May 5, 2023 •

edited

Loading