Skip to content

feat(core): Resources api change#60610

Closed
Humberd wants to merge 5 commits intoangular:mainfrom
Humberd:resources-api-change
Closed

feat(core): Resources api change#60610
Humberd wants to merge 5 commits intoangular:mainfrom
Humberd:resources-api-change

Conversation

@Humberd
Copy link
Copy Markdown
Member

@Humberd Humberd commented Mar 28, 2025

  • fix(core): getting resource value throws an error instead of returning undefined
  • fix(core): narrow error type for resources API
  • fix(core): move reload method from Resource to WritableResource
  • build: use es2022 lib

@pullapprove pullapprove bot requested review from josephperrott and mmalerba March 28, 2025 16:54
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit area: build & ci Related the build and CI infrastructure of the project area: core Issues related to the framework runtime labels Mar 28, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 28, 2025
@Humberd Humberd requested a review from alxhub March 28, 2025 16:56
@Humberd Humberd force-pushed the resources-api-change branch 2 times, most recently from ef2b989 to f5bb124 Compare March 28, 2025 17:30
@mmalerba mmalerba requested review from JeanMeche and removed request for mmalerba April 1, 2025 15:16
@Humberd Humberd force-pushed the resources-api-change branch 3 times, most recently from efd6d08 to 7825c04 Compare April 4, 2025 10:39
@atscott atscott modified the milestones: Backlog, v20 candidates Apr 4, 2025
@Humberd Humberd force-pushed the resources-api-change branch from 7825c04 to 2a9d55c Compare April 10, 2025 09:42
@Humberd Humberd force-pushed the resources-api-change branch from 2a9d55c to 0b9fe8a Compare April 17, 2025 12:43
@Humberd Humberd force-pushed the resources-api-change branch 3 times, most recently from 388387f to ad797ad Compare April 23, 2025 15:01
@Humberd
Copy link
Copy Markdown
Member Author

Humberd commented Apr 23, 2025

After the approvals were done I changed the error message from: 'Resource failed to load' to `Resource is currently in the error state'.

I also removed untracking of the stream https://github.com/angular/angular/pull/60610/files#diff-16f6b7cc4c4e69b511c8312d38c6cf71c412e6f1d6ac9092054c0f349215cdedR418, because there were some cases in g3, where hasValue was out of sync with value signal and in result was throwing errors.

@alxhub alxhub force-pushed the resources-api-change branch from ca83bd2 to f788f52 Compare April 23, 2025 23:32
@Humberd Humberd force-pushed the resources-api-change branch from f788f52 to 886c7ce Compare April 24, 2025 12:33
Copy link
Copy Markdown
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from devversion April 25, 2025 18:07
@devversion devversion removed their request for review April 29, 2025 11:33
@pullapprove pullapprove bot requested a review from devversion April 29, 2025 11:33
Copy link
Copy Markdown
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: public-api

@Humberd Humberd force-pushed the resources-api-change branch 3 times, most recently from ff4cfdc to 3161918 Compare April 30, 2025 09:40
@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Apr 30, 2025
Copy link
Copy Markdown
Member

@crisbeto crisbeto 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

Humberd added 5 commits May 6, 2025 15:16
Now only mutable resources can be reloaded

BREAKING CHANGE:
* `Resource.reload` has been moved to `WritableResource.reload`.
`Resource.error` used to return `unknown`. Now it's `Error | undefined`.
For non-`Error` types they are encapsulated with the `Error` type.

BREAKING CHANGE:
* `Resource.error` now has a type of `Signal<Error | undefined>` instead of `Signal<unknown>`
…g undefined

When there is an underlying error state it would not be possible to swallow the error with:
`computed(() => res.value()?.inner);`

BREAKING CHANGE:
* `Resource.value()` now throws an error when it's in an error state. Previously it returned `undefined`.
When the resource is loading after reloading from the error state reading `Resource.value()` would return the default value instead of throwing an error.
This change prevents `Resource.hasValue()` from throwing an error in such a case.

BREAKING CHANGE:
* `Resource.value()` now returns a default value when in a loading state after reloading the error state
@Humberd Humberd force-pushed the resources-api-change branch from 3795382 to 5e62b07 Compare May 6, 2025 13:18
@mmalerba mmalerba removed their request for review May 7, 2025 13:59
@wartab
Copy link
Copy Markdown
Contributor

wartab commented May 13, 2025

Since I was one of the people mentioning error typing during the RFC, I'm wondering about two things:

  • Why isn't the error part of the Resource generic à la Resource<T, E>?
  • Why do errors need to be instances of Error? This seems overly restrictive to me.

@JeanMeche
Copy link
Copy Markdown
Member

Changes landed as part of #61441

@JeanMeche JeanMeche closed this Jun 5, 2025
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Jul 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area: build & ci Related the build and CI infrastructure of the project area: core Issues related to the framework runtime detected: breaking change PR contains a commit with a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants