-
Notifications
You must be signed in to change notification settings - Fork 2.8k
V16: Item and Detail Base Repository should use correct typings for return types #19447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…cument-not-found-context-disconnected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request improves type safety and robustness across repository interfaces in the Umbraco.Web.UI.Client package by updating return types to use the standardized UmbRepositoryResponse and UmbRepositoryResponseWithAsObservable interfaces. Key changes include:
- Updating repository methods in tree, item, and detail repositories to return correctly typed responses.
- Replacing custom inline response objects with unified interfaces and employing optional chaining for store access.
- Adjusting repository-items manager to safely handle optional data when sorting.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository.interface.ts | Updated method return types to use UmbRepositoryResponse and UmbRepositoryResponseWithAsObservable. |
src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository-base.ts | Updated methods to use UmbRepositoryResponse and improved handling of missing treeStore with optional chaining. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/types.ts | Revised UmbRepositoryResponseWithAsObservable interface to allow for an undefined observable. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/repository-items.manager.ts | Modified private sort method to handle an undefined data array gracefully. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository.interface.ts | Updated requestItems signature to return the standardized response type. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository-base.ts | Employed optional chaining for itemStore access and updated error handling. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/detail/read/read-detail-repository.interface.ts | Adjusted requestByUnique to include the possibility of an undefined detail model. |
src/Umbraco.Web.UI.Client/src/packages/core/repository/detail/detail-repository-base.ts | Updated return type in requestByUnique and applied optional chaining for store access. |
src/Umbraco.Web.UI.Client/src/packages/content/content-type/structure/content-type-structure-manager.class.ts | Revised loadType to allow undefined detail model in the repository response. |
Comments suppressed due to low confidence (2)
src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository-base.ts:90
- Consider returning an empty observable using 'of([])' instead of undefined from asObservable to ensure consistent observable types across repository methods.
return { asObservable: () => undefined };
src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository-base.ts:44
- Consider returning an empty observable (for example, using 'of([])') instead of undefined from asObservable for consistency with the expected observable type.
return { asObservable: () => undefined };
Description
This fixes a compilation issue where some error objects were cast as
any
. It worked at runtime because the implementations sent out the correct type. This PR fixes the abstract classes to use the newer interfaces, which probably did not exist at the time they were created.Note
P.S. Don't mind the number of commits, that is because this branch was originally based on another bug fix branch, which got squashed.
Anyway, here is what Copilot thinks happened:
This pull request introduces several updates to improve type safety, enhance error handling, and simplify repository interfaces in the
Umbraco.Web.UI.Client
package. The most important changes include adding support forundefined
in repository response types, updating theasObservable
method to handle optional observables, and replacing custom response structures with standardized repository response interfaces.Enhancements to Repository Response Types:
UmbRepositoryResponseWithAsObservable
interface to support optional observables and allow specifying a different type for theasObservable
method's return value. (src/Umbraco.Web.UI.Client/src/packages/core/repository/types.ts
, src/Umbraco.Web.UI.Client/src/packages/core/repository/types.tsL14-R20)requestByUnique
,requestItems
, andrequestTreeRoot
to use the updatedUmbRepositoryResponse
andUmbRepositoryResponseWithAsObservable
interfaces. (src/Umbraco.Web.UI.Client/src/packages/core/repository/detail/detail-repository-base.ts
, [1];src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository-base.ts
, [2];src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository.interface.ts
, [3]Improved Error Handling:
undefined
as a possible type in repository responses to better handle cases where data might not be available, reducing the risk of runtime errors. (src/Umbraco.Web.UI.Client/src/packages/content/content-type/structure/content-type-structure-manager.class.ts
, [1];src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository-base.ts
, [2]Simplification of Repository Interfaces:
UmbRepositoryResponse
andUmbRepositoryResponseWithAsObservable
interfaces, streamlining the codebase. (src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository.interface.ts
, [1];src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository.interface.ts
, [2]Codebase Cleanup:
?.
) for accessing properties likeasObservable
anditems
. (src/Umbraco.Web.UI.Client/src/packages/core/repository/detail/detail-repository-base.ts
, [1];src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository-base.ts
, [2]undefined
observables. (src/Umbraco.Web.UI.Client/src/packages/core/tree/data/tree-repository-base.ts
, [1] [2]These changes collectively improve the maintainability, robustness, and type safety of the repository codebase.