Correctly generate types for Record<string, T> #1456
Conversation
When encountering a Record whose key is string, tsoa would previously generate a blank object. This change allows the type resolver to recognize the record and generate its value type properly. Intended to fix lukeautry#1407
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Best way to show this is to add a property to the Test Model here: And do assertions for the type here: and here: |
| @@ -420,6 +420,20 @@ export class TypeResolver { | |||
| return stringMetaType; | |||
| } | |||
|
|
|||
| if (typeReference.typeName.text === 'Record' && typeReference.typeArguments?.length === 2) { | |||
| const indexType = new TypeResolver(typeReference.typeArguments[0], this.current, this.parentNode, this.context).resolve(); | |||
| if (indexType.dataType === 'string') { | |||
There was a problem hiding this comment.
Why only for string? We should be able to handle numbers, boolean, string enums, number enums etc.?
There was a problem hiding this comment.
thanks for the number callout! I've added support here for numbers and test cases to verify
- number enums and string enums are handled separately (I'm not sure where, but as you can see from the test suite they are handled correctly). They actually produce fairly different OpenAPI results, because of the way swagger handles dictionaries
booleanis not a validRecordkey type; the type forRecord's key isstring | number | symbol.symbolisn't supported by tsoa it seems (which makes sense, because it doesn't belong in a public-facing API)
| if (typeReference.typeName.text === 'Record' && typeReference.typeArguments?.length === 2) { | ||
| const indexType = new TypeResolver(typeReference.typeArguments[0], this.current, this.parentNode, this.context).resolve(); | ||
| if (indexType.dataType === 'string') { | ||
| const valueType = new TypeResolver(typeReference.typeArguments[1], this.current, this.parentNode, this.context).resolve(); |
There was a problem hiding this comment.
In case we are passing from parent (GenericModel has items: Record<string, T>) can you handle these without amending the context? (IIRC that should be created on processing the GenericModel<T>, but it would also be a good test probably.
There was a problem hiding this comment.
How would I go about doing that? Should I be creating a new temporary context?
I'm not familiar with the codebase so I'm not sure what the context is here, or what purpose it serves, so I just followed the structure of other cases in this method.
There was a problem hiding this comment.
This should be handled for you at this point already.
There was a problem hiding this comment.
Sorry, I'm not sure what you mean.
- Rename the existing 'record' test into 'stringUnionRecord' to distinguish it from the new test cases - Add a 'numberUnionRecord' test case, which tests the same path as 'stringUnionRecord' - Add two new cases, 'stringRecord' and 'numberRecord', which test Record<string, T> and Record<number, T>
|
Thanks for the pointers! I've now added tests to cover my changes. |
When encountering a Record whose key is string, tsoa would previously generate a blank object. This change allows the type resolver to recognize the record and generate its value type properly.
I would love guidance on where to add some tests for this PR! The test suite is a little opaque to me.
All Submissions:
Closing issues
Closes #1407
If this is a new feature submission:
Potential Problems With The Approach
Test plan