fix(types): typescript joinArrays support #2112
Conversation
|
@marcalexiei can you verify? |
|
I’ll try to take a look tomorrow morning
On 9 Jan 2024, at 17:48, Adriano Raiano ***@***.***> wrote:
@marcalexiei<https://github.com/marcalexiei> can you verify?
—
Reply to this email directly, view it on GitHub<#2112 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AF6D2IQAWCXFXH2JKKD2DJTYNVYGVAVCNFSM6AAAAABBTJXJUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBTGQYTENBSGQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
There was a problem hiding this comment.
TypeOptions#joinArrays
joinArrays should be extracted by TOptions and not set in TypeOptions.
TypeOptions contains the global configuration properties and I do not consider joinArrays one of them since it affects single t function invocation.
The only usage is inside TReturnOptionalObjects which already has a TOpt parameter from where you can extract the joinArrays type.
All changes related to this are marked with (review main comment) comment .
I hope I got all of them 😅
Porting changes to typescript v4 file
Your changes are applied just to t.d.ts.
Could you please port your code to t.v4.d.ts?
| it('should work with `count`', () => { | ||
| expectTypeOf(t('key', { count: 1 })).toEqualTypeOf<'item' | 'items'>(); | ||
| }); |
There was a problem hiding this comment.
I don't think this test should go here. key values are just strings:
// ...
key_one: 'item';
key_other: 'items';
// ...
This scenario should be tested in misc or custom-types workspaces, if it is not already.
|
|
||
| /** | ||
| * A string to separate each pair of adjacent elements of the array | ||
| */ | ||
| joinArrays: false; |
There was a problem hiding this comment.
This should be removed (review main comment)
| @@ -11,6 +17,7 @@ import type { | |||
| /* eslint @typescript-eslint/ban-types: ['error', { types: { "{}": false } }] */ | |||
|
|
|||
| // Type Options | |||
| type _JoinArrays = TypeOptions['joinArrays']; | |||
There was a problem hiding this comment.
This should be removed (review main comment)
|
|
||
| type TReturnOptionalNull = _ReturnNull extends true ? null : never; | ||
| type TReturnOptionalObjects<TOpt extends TOptions> = _ReturnObjects extends true | ||
| type TReturnOptionalObjects<TOpt extends TOptions> = _JoinArrays extends string |
There was a problem hiding this comment.
This should be change to rely on TOpt type parameter (review main comment)
| type TReturnOptionalObjects<TOpt extends TOptions> = _JoinArrays extends string | |
| type TReturnOptionalObjects<TOpt extends TOptions> = TOpt['joinArrays'] extends string |
|
@sreucherand any update on this? |
|
Hi @adrai , I'll get back on this asap, sorry about the delay. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
@sreucherand any update on this? |
This fixes typing errors when using joinArrays on array types.
I'm not completely confident on the approach but I did my best to define a clear test suite that would demonstrate the issues I encountered. I'd be glad to discuss this through.
Here is a minimum reproducible example of the initial problem: https://codesandbox.io/p/sandbox/18n-basic-example-2-forked-7lgxrf?file=/src/App.tsx:10,11
Checklist
npm run test