-
Notifications
You must be signed in to change notification settings - Fork 4
Fix: use explicit types instead of interface inheritance for hooks #44
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
Conversation
Revert commit 6849141 Replace interface inheritance pattern with explicit type definitions to ensure TypeScript declarations bundle correctly. The StacHook base interface was causing vite-plugin-dts to generate incomplete type declarations where inherited properties (isLoading, isFetching, error) were not visible to library consumers. Changes: - Remove StacHook interface and StacRefetchFn<T> from src/types/index.d.ts - Convert all hook return types from interface inheritance to explicit types - Add all properties directly to each hook type definition - Fix error type coercion in useStacSearch (undefined → null) - Fixes "Property 'isLoading' does not exist" errors in consuming projects
| isFetching: boolean; | ||
| refetch: () => Promise<QueryObserverResult<Collection, ApiError>>; | ||
| error: ApiError | null; | ||
| }; |
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.
could extend with types too like
type StacHook = {
isLoading: boolean;
isFetching: boolean;
error: ApiError | null;
};
type StacCollectionHook = {
refetch: () => Promise<QueryObserverResult<Collection, ApiError>>;
} & StacHook
sandrahoang686
left a comment
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.
good to know for bundling!
|
|
||
| /** | ||
| * Base interface for all STAC hooks providing common loading state and error handling. | ||
| * All data-fetching hooks (useCollection, useCollections, useItem, useStacSearch) | ||
| * extend this interface with their specific data and refetch signatures. | ||
| */ | ||
| export interface StacHook { | ||
| /** True during initial data fetch (no cached data available) */ | ||
| isLoading: boolean; | ||
| /** True during any fetch operation (including background refetches) */ | ||
| isFetching: boolean; | ||
| /** Error information if the last request was unsuccessful */ | ||
| error: ApiError | null; | ||
| } | ||
|
|
||
| /** | ||
| * Generic refetch function type for STAC hooks. | ||
| * Returns a Promise with the query result including data and error information. | ||
| */ | ||
| export type StacRefetchFn<T> = () => Promise<QueryObserverResult<T, ApiError>>; |
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.
It is possible to keep this pattern and have the types correctly exported. Was there any reason not to?
| isLoading, | ||
| isFetching, | ||
| error, | ||
| error: error ?? null, |
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.
The error is already null. Why is this needed?
|
@sandrahoang686 @AliceR Made a proposal fix in this PR (#45) that doesn't remove the inheritance and reduced duplication. |
Revert commit 6849141
Replace interface inheritance pattern with explicit type definitions to ensure TypeScript declarations bundle correctly. The StacHook base interface was causing vite-plugin-dts to generate incomplete type declarations where inherited properties (isLoading, isFetching, error) were not visible to library consumers.
Changes:
Fixes "Property 'isLoading' does not exist" errors in consuming projects