Fix implementation of nonOptional, nonNullable and nonNullish #909

fabian-hiller · Nov 10, 2024 · aca6266 · aca6266
1 parent d23a2c6
commit aca6266
Show file tree

Hide file tree

Showing 16 changed files with 302 additions and 74 deletions.
diff --git a/library/CHANGELOG.md b/library/CHANGELOG.md
@@ -16,6 +16,7 @@ All notable changes to the library will be documented in this file.
 - Change type signature of `partialCheck` and `partialCheckAsync` action to add `.pathList` property in a type-safe way
 - Change type signature of `findItem` action to support type predicates (issue #867)
 - Refactor `bytes`, `maxBytes`, `minBytes` and `notBytes` action
+- Fix implementation of `nonOptional`, `nonOptionalAsync`, `nonNullable`, `nonNullableAsync`, `nonNullish` and `nonNullishAsync` schema in edge cases (issue #909)
 
 ## v0.42.1 (September 20, 2024)
 

diff --git a/library/src/schemas/nonNullable/nonNullable.test.ts b/library/src/schemas/nonNullable/nonNullable.test.ts
@@ -1,4 +1,7 @@
 import { describe, expect, test } from 'vitest';
+import { transform } from '../../actions/index.ts';
+import { pipe } from '../../methods/index.ts';
+import type { FailureDataset } from '../../types/index.ts';
 import { expectNoSchemaIssue, expectSchemaIssue } from '../../vitest/index.ts';
 import { nullish, type NullishSchema } from '../nullish/index.ts';
 import { string, type StringSchema } from '../string/index.ts';
@@ -72,16 +75,43 @@ describe('nonNullable', () => {
   });
 
   describe('should return dataset with issues', () => {
-    const schema = nonNullable(nullish(string()), 'message');
-    const baseIssue: Omit<NonNullableIssue, 'input' | 'received'> = {
+    const nonNullableIssue: NonNullableIssue = {
       kind: 'schema',
       type: 'non_nullable',
+      input: null,
+      received: 'null',
       expected: '!null',
       message: 'message',
+      requirement: undefined,
+      path: undefined,
+      issues: undefined,
+      lang: undefined,
+      abortEarly: undefined,
+      abortPipeEarly: undefined,
     };
 
-    test('for null', () => {
-      expectSchemaIssue(schema, baseIssue, [null]);
+    test('for null input', () => {
+      expectSchemaIssue(
+        nonNullable(nullish(string()), 'message'),
+        nonNullableIssue,
+        [null]
+      );
+    });
+
+    test('for null output', () => {
+      expect(
+        nonNullable(
+          pipe(
+            string(),
+            transform(() => null)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: null,
+        issues: [nonNullableIssue],
+      } satisfies FailureDataset<NonNullableIssue>);
     });
   });
 });
diff --git a/library/src/schemas/nonNullable/nonNullable.ts b/library/src/schemas/nonNullable/nonNullable.ts
@@ -2,7 +2,7 @@ import type {
   BaseIssue,
   BaseSchema,
   ErrorMessage,
-  FailureDataset,
+  OutputDataset,
 } from '../../types/index.ts';
 import { _addIssue, _getStandardProps } from '../../utils/index.ts';
 import type {
@@ -88,15 +88,19 @@ export function nonNullable(
       return _getStandardProps(this);
     },
     '~run'(dataset, config) {
-      // If value is `null`, add issue and return dataset
+      // If value is not `null`, run wrapped schema
+      if (dataset.value !== null) {
+        this.wrapped['~run'](dataset, config);
+      }
+
+      // If value is `null`, add issue to dataset
       if (dataset.value === null) {
         _addIssue(this, 'type', dataset, config);
-        // @ts-expect-error
-        return dataset as FailureDataset<NonNullableIssue>;
       }
 
-      // Otherwise, return dataset of wrapped schema
-      return this.wrapped['~run'](dataset, config);
+      // Return output dataset
+      // @ts-expect-error
+      return dataset as OutputDataset<unknown, BaseIssue<unknown>>;
     },
   };
 }
diff --git a/library/src/schemas/nonNullable/nonNullableAsync.test.ts b/library/src/schemas/nonNullable/nonNullableAsync.test.ts
@@ -1,4 +1,7 @@
 import { describe, expect, test } from 'vitest';
+import { transform } from '../../actions/index.ts';
+import { pipeAsync } from '../../methods/index.ts';
+import type { FailureDataset } from '../../types/index.ts';
 import {
   expectNoSchemaIssueAsync,
   expectSchemaIssueAsync,
@@ -78,16 +81,43 @@ describe('nonNullableAsync', () => {
   });
 
   describe('should return dataset with issues', () => {
-    const schema = nonNullableAsync(nullishAsync(string()), 'message');
-    const baseIssue: Omit<NonNullableIssue, 'input' | 'received'> = {
+    const nonNullableIssue: NonNullableIssue = {
       kind: 'schema',
       type: 'non_nullable',
+      input: null,
+      received: 'null',
       expected: '!null',
       message: 'message',
+      requirement: undefined,
+      path: undefined,
+      issues: undefined,
+      lang: undefined,
+      abortEarly: undefined,
+      abortPipeEarly: undefined,
     };
 
-    test('for null', async () => {
-      await expectSchemaIssueAsync(schema, baseIssue, [null]);
+    test('for null input', async () => {
+      await expectSchemaIssueAsync(
+        nonNullableAsync(nullishAsync(string()), 'message'),
+        nonNullableIssue,
+        [null]
+      );
+    });
+
+    test('for null output', async () => {
+      expect(
+        await nonNullableAsync(
+          pipeAsync(
+            string(),
+            transform(() => null)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: null,
+        issues: [nonNullableIssue],
+      } satisfies FailureDataset<NonNullableIssue>);
     });
   });
 });
diff --git a/library/src/schemas/nonNullable/nonNullableAsync.ts b/library/src/schemas/nonNullable/nonNullableAsync.ts
@@ -3,7 +3,7 @@ import type {
   BaseSchema,
   BaseSchemaAsync,
   ErrorMessage,
-  FailureDataset,
+  OutputDataset,
 } from '../../types/index.ts';
 import { _addIssue, _getStandardProps } from '../../utils/index.ts';
 import type {
@@ -101,15 +101,19 @@ export function nonNullableAsync(
       return _getStandardProps(this);
     },
     async '~run'(dataset, config) {
-      // If value is `null`, add issue and return dataset
+      // If value is not `null`, run wrapped schema
+      if (dataset.value !== null) {
+        await this.wrapped['~run'](dataset, config);
+      }
+
+      // If value is `null`, add issue to dataset
       if (dataset.value === null) {
         _addIssue(this, 'type', dataset, config);
-        // @ts-expect-error
-        return dataset as FailureDataset<NonNullableIssue>;
       }
 
-      // Otherwise, return dataset of wrapped schema
-      return this.wrapped['~run'](dataset, config);
+      // Return output dataset
+      // @ts-expect-error
+      return dataset as OutputDataset<unknown, BaseIssue<unknown>>;
     },
   };
 }
diff --git a/library/src/schemas/nonNullable/types.ts b/library/src/schemas/nonNullable/types.ts
@@ -50,10 +50,7 @@ export type InferNonNullableOutput<
   TWrapped extends
     | BaseSchema<unknown, unknown, BaseIssue<unknown>>
     | BaseSchemaAsync<unknown, unknown, BaseIssue<unknown>>,
-> =
-  // FIXME: For schemas that transform the input to `null`, this
-  // implementation may result in an incorrect output type
-  NonNullable<InferOutput<TWrapped>>;
+> = NonNullable<InferOutput<TWrapped>>;
 
 /**
  * Infer non nullable issue type.

diff --git a/library/src/schemas/nonNullish/nonNullish.test.ts b/library/src/schemas/nonNullish/nonNullish.test.ts
@@ -1,4 +1,7 @@
 import { describe, expect, test } from 'vitest';
+import { transform } from '../../actions/index.ts';
+import { pipe } from '../../methods/index.ts';
+import type { FailureDataset } from '../../types/index.ts';
 import { expectNoSchemaIssue, expectSchemaIssue } from '../../vitest/index.ts';
 import { nullish, type NullishSchema } from '../nullish/index.ts';
 import { string, type StringSchema } from '../string/index.ts';
@@ -72,20 +75,61 @@ describe('nonNullish', () => {
   });
 
   describe('should return dataset with issues', () => {
-    const schema = nonNullish(nullish(string()), 'message');
     const baseIssue: Omit<NonNullishIssue, 'input' | 'received'> = {
       kind: 'schema',
       type: 'non_nullish',
       expected: '(!null & !undefined)',
       message: 'message',
+      requirement: undefined,
+      path: undefined,
+      issues: undefined,
+      lang: undefined,
+      abortEarly: undefined,
+      abortPipeEarly: undefined,
     };
 
-    test('for null', () => {
-      expectSchemaIssue(schema, baseIssue, [null]);
+    test('for null input', () => {
+      expectSchemaIssue(nonNullish(nullish(string()), 'message'), baseIssue, [
+        null,
+      ]);
     });
 
-    test('for undefined', () => {
-      expectSchemaIssue(schema, baseIssue, [undefined]);
+    test('for undefined input', () => {
+      expectSchemaIssue(nonNullish(nullish(string()), 'message'), baseIssue, [
+        undefined,
+      ]);
+    });
+
+    test('for null output', () => {
+      expect(
+        nonNullish(
+          pipe(
+            string(),
+            transform(() => null)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: null,
+        issues: [{ ...baseIssue, input: null, received: 'null' }],
+      } satisfies FailureDataset<NonNullishIssue>);
+    });
+
+    test('for undefined output', () => {
+      expect(
+        nonNullish(
+          pipe(
+            string(),
+            transform(() => undefined)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: undefined,
+        issues: [{ ...baseIssue, input: undefined, received: 'undefined' }],
+      } satisfies FailureDataset<NonNullishIssue>);
     });
   });
 });
diff --git a/library/src/schemas/nonNullish/nonNullish.ts b/library/src/schemas/nonNullish/nonNullish.ts
@@ -2,7 +2,7 @@ import type {
   BaseIssue,
   BaseSchema,
   ErrorMessage,
-  FailureDataset,
+  OutputDataset,
 } from '../../types/index.ts';
 import { _addIssue, _getStandardProps } from '../../utils/index.ts';
 import type {
@@ -88,15 +88,19 @@ export function nonNullish(
       return _getStandardProps(this);
     },
     '~run'(dataset, config) {
-      // If value is `null` or `undefined`, add issue and return dataset
+      // If value is not `null` and `undefined`, run wrapped schema
+      if (!(dataset.value === null || dataset.value === undefined)) {
+        this.wrapped['~run'](dataset, config);
+      }
+
+      // If value is `null` or `undefined`, add issue to dataset
       if (dataset.value === null || dataset.value === undefined) {
         _addIssue(this, 'type', dataset, config);
-        // @ts-expect-error
-        return dataset as FailureDataset<NonNullishIssue>;
       }
 
-      // Otherwise, return dataset of wrapped schema
-      return this.wrapped['~run'](dataset, config);
+      // Return output dataset
+      // @ts-expect-error
+      return dataset as OutputDataset<unknown, BaseIssue<unknown>>;
     },
   };
 }
diff --git a/library/src/schemas/nonNullish/nonNullishAsync.test.ts b/library/src/schemas/nonNullish/nonNullishAsync.test.ts
@@ -1,4 +1,7 @@
 import { describe, expect, test } from 'vitest';
+import { transform } from '../../actions/index.ts';
+import { pipeAsync } from '../../methods/index.ts';
+import type { FailureDataset } from '../../types/index.ts';
 import {
   expectNoSchemaIssueAsync,
   expectSchemaIssueAsync,
@@ -78,20 +81,65 @@ describe('nonNullishAsync', () => {
   });
 
   describe('should return dataset with issues', () => {
-    const schema = nonNullishAsync(nullishAsync(string()), 'message');
     const baseIssue: Omit<NonNullishIssue, 'input' | 'received'> = {
       kind: 'schema',
       type: 'non_nullish',
       expected: '(!null & !undefined)',
       message: 'message',
+      requirement: undefined,
+      path: undefined,
+      issues: undefined,
+      lang: undefined,
+      abortEarly: undefined,
+      abortPipeEarly: undefined,
     };
 
-    test('for null', async () => {
-      await expectSchemaIssueAsync(schema, baseIssue, [null]);
+    test('for null input', async () => {
+      await expectSchemaIssueAsync(
+        nonNullishAsync(nullishAsync(string()), 'message'),
+        baseIssue,
+        [null]
+      );
     });
 
-    test('for undefined', async () => {
-      await expectSchemaIssueAsync(schema, baseIssue, [undefined]);
+    test('for undefined input', async () => {
+      await expectSchemaIssueAsync(
+        nonNullishAsync(nullishAsync(string()), 'message'),
+        baseIssue,
+        [undefined]
+      );
+    });
+
+    test('for null output', async () => {
+      expect(
+        await nonNullishAsync(
+          pipeAsync(
+            string(),
+            transform(() => null)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: null,
+        issues: [{ ...baseIssue, input: null, received: 'null' }],
+      } satisfies FailureDataset<NonNullishIssue>);
+    });
+
+    test('for undefined output', async () => {
+      expect(
+        await nonNullishAsync(
+          pipeAsync(
+            string(),
+            transform(() => undefined)
+          ),
+          'message'
+        )['~run']({ value: 'foo' }, {})
+      ).toStrictEqual({
+        typed: false,
+        value: undefined,
+        issues: [{ ...baseIssue, input: undefined, received: 'undefined' }],
+      } satisfies FailureDataset<NonNullishIssue>);
     });
   });
 });