Skip to content

Conversation

@alxhub
Copy link
Member

@alxhub alxhub commented Jan 24, 2025

2 fixes for resource()

When a resource first starts up, even if it transitions immediately to
`Loading` it should report a `previous.state` of `Idle`. It was reporting
`Loading` as the previous state in such a case because of an oversight in
the migration to `linkedSignal` which this commit addresses.
`hasValue` attempts to narrow the type of a resource to exclude `undefined`.
Because of the way signal types merge in TS, this only works if the type
of the resource is the same general type as `hasValue` asserts.

For example, if `res` is `WritableResource<string|undefined>` then
`.hasValue()` correctly asserts that `res` is `WritableResource<string>` and
`.value()` will be narrowed. If `res` is `ResourceRef<string|undefined>`
then that narrowing does _not_ work correctly, since `.hasValue()` will
assert `res` is `WritableResource<string>` and TS will combine that for a
final type of `ResourceRef<string|undefined> & WritableResource<string>`.
The final type of `.value()` then will not narrow.

This commit fixes the above problem by adding a `.hasValue()` override to
`ResourceRef` which asserts the resource is of type `ResourceRef`.

Fixes angular#59707
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Jan 24, 2025
@ngbot ngbot bot added this to the Backlog milestone Jan 24, 2025
Copy link
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

* @experimental
*/
export interface ResourceRef<T> extends WritableResource<T> {
hasValue(): this is ResourceRef<Exclude<T, undefined>>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe capture this in a test ?

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@pkozlowski-opensource pkozlowski-opensource removed their request for review January 27, 2025 16:00
@alxhub alxhub added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Jan 28, 2025
@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit 6c92d65.

The changes were merged into the following branches: main

pkozlowski-opensource pushed a commit that referenced this pull request Jan 28, 2025
`hasValue` attempts to narrow the type of a resource to exclude `undefined`.
Because of the way signal types merge in TS, this only works if the type
of the resource is the same general type as `hasValue` asserts.

For example, if `res` is `WritableResource<string|undefined>` then
`.hasValue()` correctly asserts that `res` is `WritableResource<string>` and
`.value()` will be narrowed. If `res` is `ResourceRef<string|undefined>`
then that narrowing does _not_ work correctly, since `.hasValue()` will
assert `res` is `WritableResource<string>` and TS will combine that for a
final type of `ResourceRef<string|undefined> & WritableResource<string>`.
The final type of `.value()` then will not narrow.

This commit fixes the above problem by adding a `.hasValue()` override to
`ResourceRef` which asserts the resource is of type `ResourceRef`.

Fixes #59707

PR Close #59708
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
When a resource first starts up, even if it transitions immediately to
`Loading` it should report a `previous.state` of `Idle`. It was reporting
`Loading` as the previous state in such a case because of an oversight in
the migration to `linkedSignal` which this commit addresses.

PR Close angular#59708
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
`hasValue` attempts to narrow the type of a resource to exclude `undefined`.
Because of the way signal types merge in TS, this only works if the type
of the resource is the same general type as `hasValue` asserts.

For example, if `res` is `WritableResource<string|undefined>` then
`.hasValue()` correctly asserts that `res` is `WritableResource<string>` and
`.value()` will be narrowed. If `res` is `ResourceRef<string|undefined>`
then that narrowing does _not_ work correctly, since `.hasValue()` will
assert `res` is `WritableResource<string>` and TS will combine that for a
final type of `ResourceRef<string|undefined> & WritableResource<string>`.
The final type of `.value()` then will not narrow.

This commit fixes the above problem by adding a `.hasValue()` override to
`ResourceRef` which asserts the resource is of type `ResourceRef`.

Fixes angular#59707

PR Close angular#59708
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants