Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation should fail when required property (which may be undefined) is missing #596

Open
OliverJAsh opened this issue Jun 17, 2021 · 5 comments

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Jun 17, 2021

🐛 Bug report

Current Behavior

const t = require('io-ts')
const result = t.type({ foo: t.string, bar: t.union([t.undefined, t.string])}).decode({ foo: 'foo' })
console.log({result});
// { result: { _tag: 'Right', right: { foo: 'foo', bar: undefined } } }

Expected behavior

I expected it to return a Left to match the TS behaviour:

// Errors as expected
// Property 'bar' is missing in type '{ foo: string; }' but required in type '{ foo: string; bar: string | undefined; }'.ts(2741)
const result: { foo: string; bar: string | undefined } = { foo: 'foo' };

This is because bar is defined as a required property which must be explicitly set as either string or undefined.

Reproducible example

See above.

Suggested solution(s)

Additional context

Your environment

Which versions of io-ts are affected by this issue? Did this work in previous versions of io-ts?

Software Version(s)
io-ts
fp-ts
TypeScript
@samhh
Copy link

samhh commented Jun 17, 2021

Potentially relevant:

@SHaTRO
Copy link

SHaTRO commented Sep 25, 2021

Semantically speaking "undefined" and not defined are supposed to be equivalent. Requiring "undefined" to define something as "undefined" is literally a contradiction. That is why missing and undefined have been treated as semantically equivalent - because they are.

If you need to define something as having no value, then first it must be a thing (defined) and secondly must be defined as having no value (also defined). That is what "null" is for. The reason we frown on "null" is because it does this: defines a property as having no value.

Now, if we were to say, change our semantics to "declare" that a property is undefined - we are still defining it when we declare it, but we are "defining" it as "undefined" which is a contradiction. Folks who insist on doing this should be using "null" to distinguish between "defined as not having a value" and "undefined" (which means missing/empty/void).

I believe this interpretation is completely consistent with --strictOptionalProperties


TL;DR - to say that a "required" condition is fulfilled by declaring something as "undefined" is semantically incorrect. Something that is declared as "undefined" has also been declared as "missing".

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Dec 1, 2021

Semantically speaking "undefined" and not defined are supposed to be equivalent.

@SHaTRO I'm not sure if they are equivalent in terms of behaviour, because the in operator will produce different results when the key doesn't exist versus when it does exist but the value is set to undefined:

type A = { foo: string | undefined }
const a: A = { foo: undefined };
'foo' in a; // true

type B = { foo?: string }
const b: A = {};
'foo' in b; // false

I'm not trying to imply it's a good idea to use "explicit undefined"s, rather I'm just pointing out that the behaviour in io-ts doesn't match the behaviour in TypeScript.

Say we have this type:

const t = require('io-ts')
const I = t.type({ foo: t.string, bar: t.union([t.undefined, t.string])})

It is reasonable to expect that, after a successful decode, 'bar' in value will produce true only if the key existed on the input value, because we have declared the key as required.

However this is not currently the case. In this example, the decode succeeds even though the required key is missing:

const result = I.decode({ foo: 'foo' })
console.log({result});
// { result: { _tag: 'Right', right: { foo: 'foo', bar: undefined } } }
console.log('bar' in result)
// => true
// Key 'bar' never existed in the input value ❗️

OTOH, if we model the type using a TypeScript type instead, the assignment fails:

// Errors as expected
// Property 'bar' is missing in type '{ foo: string; }' but required in type '{ foo: string; bar: string | undefined; }'.ts(2741)
const result: { foo: string; bar: string | undefined } = { foo: 'foo' };

@ragnese
Copy link

ragnese commented Dec 8, 2021

@SHaTRO
Unfortunately, I have to agree with @OliverJAsh. TypeScript has made the (questionable, IMO) decision that there is a distinct difference between having a field with a value of undefined and a field not being present/assigned.

I'm sure we're all aware of the following, but I'll add it just to be clear:

interface OptionalField {
    field?: number
}
interface RequiredField {
    field: number | undefined
}
declare const o: OptionalField
declare const r: RequiredField

let x: OptionalField = o
let y: RequiredField = r

x = y // Okay
y = x // Error

In light of this, I think the behavior suggested by this issue would be more correct (albeit very tedious and usually not what a user of this library probably wants most of the time).

Perhaps it would be valuable to include (in addition to the suggestion in OP) an optional() wrapper that works like partial({}), but only for individual fields of a Props definition, rather than needing to intersect your non-partial fields with your partial fields to get the desired effect.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Dec 13, 2021

I just realised that this also means the following codec law does not hold:

import * as assert from "assert";
import * as E from "fp-ts/Either";
import { pipe } from "fp-ts/function";
import * as t from "io-ts";

const codec = t.type({
    foo: t.undefined,
});

const u = {};

// ❌ This fails
assert.deepEqual(
    pipe(
        codec.decode(u),
        E.fold(() => u, codec.encode)
    ),
    u
);
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  foo: undefined
}

should loosely deep-equal

{}

Here's a more practical example that uses optionFromNullable:

import * as assert from "assert";
import * as E from "fp-ts/Either";
import { pipe } from "fp-ts/function";
import * as t from "io-ts";
import { optionFromNullable } from "io-ts-types";

const codec = t.type({
    foo: optionFromNullable(t.string),
});

const u = {};

// ❌ This fails
assert.deepEqual(
    pipe(
        codec.decode(u),
        E.fold(() => u, codec.encode)
    ),
    u
);
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  foo: null
}

should loosely deep-equal

{}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants