Improve types for ParameterSettings #42269
Conversation
| return [sectionId, defaultOption]; | ||
| }), | ||
| ); | ||
|
|
||
| return map; | ||
| return map as Record<SectionId, ParameterMappingOptions>; |
There was a problem hiding this comment.
I used as here because Object.fromEntries returns <string, T> and not <SectionId, T>
There was a problem hiding this comment.
Actually Object.fromEntries returns (roughly) Partial<Record<SectionId, ParameterMappingOptions>>. TS can't ensure that every SectionId key will be present.
There was a problem hiding this comment.
🤔 maybe I have wrong TS version locally
There was a problem hiding this comment.
My point was that if you remove the cast and explicitly specify that this function returns Partial<Record<SectionId, ParameterMappingOptions>> - TS won't complain (in this file at least).
There was a problem hiding this comment.
true, I reworked it a bit e387a47
|
| @@ -64,7 +64,7 @@ function getParameterOperatorType(parameterType?: string) { | |||
|
|
|||
| export function buildTypedOperatorOptions( | |||
| operatorType: OperatorType, | |||
| sectionId: string, | |||
| sectionId: "number" | "string" | "date" | "location" | "id", | |||
There was a problem hiding this comment.
| sectionId: "number" | "string" | "date" | "location" | "id", | |
| sectionId: SectionId, |
There was a problem hiding this comment.
unfortunately imports from main codebase is restricted in metabase-lib folder
There was a problem hiding this comment.
Is there any other place where we can define SectionId to not hit this problem?
There was a problem hiding this comment.
inside the metabase-lib 😄 let me fix it
There was a problem hiding this comment.
| name: t`Number`, | ||
| description: t`Subtotal, Age, Price, Quantity, etc.`, | ||
| options: buildTypedOperatorOptions("number", "number", t`Number`), | ||
| }, | ||
| { | ||
| id: "string", | ||
| id: "string" as const, | ||
| name: t`Text or Category`, | ||
| description: t`Name, Rating, Description, etc.`, | ||
| options: buildTypedOperatorOptions("string", "string", t`Text`), | ||
| }, | ||
| ].filter(Boolean); |
There was a problem hiding this comment.
This filter is redundant
| return [sectionId, defaultOption]; | ||
| }), | ||
| ); | ||
|
|
||
| return map; | ||
| return map as Record<SectionId, ParameterMappingOptions>; |
There was a problem hiding this comment.
Actually Object.fromEntries returns (roughly) Partial<Record<SectionId, ParameterMappingOptions>>. TS can't ensure that every SectionId key will be present.
closes #42196
Provide better types instead of general
stringforSectionId.How to verify