Add NonEmptyString type#1103
Conversation
test-d/non-empty-string.ts
Outdated
| declare const a: NonEmptyString<'a'>; | ||
| expectType<NonEmptyString<'a'>>(a); | ||
|
|
||
| declare const b: NonEmptyString<'b' | 'c'>; | ||
| expectType<NonEmptyString<'b' | 'c'>>(b); |
There was a problem hiding this comment.
The expected type and the actual type are identical, making these tests useless.
| declare const a: NonEmptyString<'a'>; | |
| expectType<NonEmptyString<'a'>>(a); | |
| declare const b: NonEmptyString<'b' | 'c'>; | |
| expectType<NonEmptyString<'b' | 'c'>>(b); | |
| declare const a: NonEmptyString<'a'>; | |
| expectType<'a'>(a); | |
| declare const b: NonEmptyString<'b' | 'c'>; | |
| expectType<'b' | 'c'>(b); |
test-d/non-empty-string.ts
Outdated
| declare const a: NonEmptyString<'a'>; | ||
| expectType<NonEmptyString<'a'>>(a); | ||
|
|
||
| declare const b: NonEmptyString<'b' | 'c'>; | ||
| expectType<NonEmptyString<'b' | 'c'>>(b); |
There was a problem hiding this comment.
Also, add these test cases:
expectType<'a'>({} as NonEmptyString<'' | 'a'>);
expectType<string>({} as NonEmptyString<string>);
expectType<Uppercase<string>>({} as NonEmptyString<Uppercase<string>>);
expectType<`on${string}`>({} as NonEmptyString<`on${string}`>);
test-d/non-empty-string.ts
Outdated
| declare const empty: NonEmptyString<''>; | ||
| expectNever(empty); |
There was a problem hiding this comment.
Better to use expectType instead of expectNever.
| declare const empty: NonEmptyString<''>; | |
| expectNever(empty); | |
| declare const empty: NonEmptyString<''>; | |
| expectType<never>(empty); |
|
I would like to see some more tests. Think of more edge-cases to test. Here are some AI-generated suggestions: // Tests that string types from mapped types work correctly
type StringMap = { [K in 'a' | 'b']: string };
type StringMapValues = StringMap[keyof StringMap];
expectType<string>({} as NonEmptyString<StringMapValues>);
// Tests handling of conditional types that resolve to empty string or string
type MaybeEmpty<T> = T extends number ? '' : string;
expectType<never>({} as NonEmptyString<MaybeEmpty<number>>);
expectType<string>({} as NonEmptyString<MaybeEmpty<boolean>>);
// Tests distribution over unions in conditional types
type UnionResult<T> = T extends string ? T | '' : never;
expectType<string>({} as NonEmptyString<UnionResult<string>>);
// Tests with generic constraints - this shows behavior with type parameters
declare function genericTest<T extends string, U extends ''>(
a: NonEmptyString<T>,
b: NonEmptyString<U>
): void; |
@sindresorhus These AI generated tests aren't actually doing anything different—they just look complicated. Given that the tests suggested here are added. Let's break them down:
This is effectively just
This just tests
Again, this simplifies down to
And this one doesn't seem to test anything at all. The only meaningful edge case I can think of is testing for generic assignability. // `NonEmptyString<S>` should be assignable to `string`
type Assignability1<_S extends string> = unknown;
type Test1<S extends string> = Assignability1<NonEmptyString<S>>;
// `string` should NOT be assignable to `NonEmptyString<S>`
type Assignability2<_S extends string, _SS extends NonEmptyString<_S>> = unknown;
// @ts-expect-error
type Test2<S extends string> = Assignability2<S, S>; |
|
On second thought, I believe we should return // If `string_` is typed as `NonEmptyString<T>`, the implementation of `foo` likely assumes
// that `string_` should never be an empty string at runtime.
declare function foo<T extends string>(string_: NonEmptyString<T>): void;
declare const someString: string;
foo(someString); // No error, because `NonEmptyString<string>` is just `string`,
// but `someString` could be an empty string at runtime.And this behavior is also inconsistent with the following: declare const aOrBOrEmpty: "a" | "b" | "";
foo(aOrBOrEmpty); // Errors
// Argument of type '"" | "a" | "b"' is not assignable to parameter of type '"a" | "b"'.
// Type '""' is not assignable to type '"a" | "b"'.If @sindresorhus @jor1ken WDYT? |
👍 |
72b1ed2 to
78ef575
Compare
som-sm
left a comment
There was a problem hiding this comment.
@sindresorhus I've updated the implementation to handle this and also updated the tests, please have a look!
closed #946