-
Notifications
You must be signed in to change notification settings - Fork 119
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
refactor: improve typing of sorting in facetGetters/category.vue #1080
Conversation
import type { UseFacetSearchResult } from '~/modules/catalog/category/composables/useFacet/useFacet'; | ||
|
||
export interface FacetsGetters { | ||
getSortOptions: (searchData: UseFacetSearchResult) => SortingModel; |
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.
Previously the type here was AgnosticSort
which turned out to be completely different at runtime. TypeScript didn't report an error since the searchData argument was any.
}; | ||
}; | ||
|
||
const getProducts = (searchData): ProductInterface[] => { |
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.
I inlined the methods into the exported asserted interface since I didn't want to duplicate the typedefs
@@ -1,3 +1,13 @@ | |||
export interface SortingModel { |
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.
SortingModel as to avoid having "SortingOptions" and "SortingOption" interfaces with similar names
M2-645 related to M2-524 since it removes AgnosticSort
9f7d1fc
to
68b0ba7
Compare
} as AgnosticSort; | ||
} | ||
options: searchData.data.availableSortingOptions, | ||
selected: searchData.input.sort, |
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.
What if there will be no sort param?
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.
CategoryNavbar will not show any sort option. This will only happen if someone puts a sort id that doesn't exist into the url.
Fix issue where AgnosticSort was totally different type than what would appear at runtime