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

strictCheck allows extra properties from siblings in union, sometimes #51

Open
alexmojaki opened this issue Apr 14, 2021 · 0 comments
Open

Comments

@alexmojaki
Copy link
Contributor

This test fails:

  it("should handle strict unions", () => {
    const {ab, ac, union, intersection} = createCheckers({
      ab: t.iface([], {"a": "number", "b": "number"}),
      ac: t.iface([], {"a": "number", "c": "number"}),
      union: t.union("ab", "ac"),
      intersection: t.intersection("union"),
    });
    const value = {a: 1, b: 2, c: 3};
    assert.isTrue(ab.test(value));
    assert.isTrue(ac.test(value));
    assert.isTrue(union.test(value));
    assert.isFalse(ab.strictTest(value));
    assert.isFalse(ac.strictTest(value));
    assert.isFalse(union.strictTest(value));
    assert.isFalse(intersection.strictTest(value));
  });

Most of this is fine. value satisfies ab and ac, so it satisfies the union, but it doesn't satisfy either strictly so it doesn't satisfy the union either. However the implementation is fragile and only just manages to get assert.isFalse(union.strictTest(value)) right. The last line assert.isFalse(intersection.strictTest(value)) fails and reveals the problem. When the union is inside an intersection, its children checkers for ab and ac pass the strict test for value.

This is related to #45.

What happens is that in these lines in TUnion:

public getChecker(suite: ITypeSuite, strict: boolean, allowedProps?: Set<string>): CheckerFunc {
const itemCheckers = this.ttypes.map((t) => t.getChecker(suite, strict, allowedProps));

allowedProps is passed to TIface:

public getChecker(suite: ITypeSuite, strict: boolean, allowedProps: Set<string> = new Set()): CheckerFunc {
this.propSet.forEach((prop) => allowedProps.add(prop));

which means that back in TUnion allowedProps becomes { 'a', 'b', 'c' } (confirmed by logging) after being populated by the checkers for both ab and ac.

However this doesn't happen if you check the union by itself. Since allowedProps is optional and has no default in TUnion.getChecker, it's just undefined, and each TIface makes its own separate set by default. But once the union is in an intersection, the intersection makes a new set which gets passed down and shared between the interfaces.

This is complicated so I haven't figured out how to solve it, but it seems that you would need to have multiple sets of allowed props and an interface would need to check that the properties fall within at least one of those sets. To put this example in concrete TS:

    interface AB {
      a: number,
      b: number,
    }
    interface AC {
      a: number,
      c: number,
    }
    type U = AB | AC
    interface D {
      d: number
    }
    type I = U & D

    // These work:
    const i: I = {a: 1, b: 1, d: 1}
    const i: I = {a: 1, c: 1, d: 1}

    // but this doesn't:
    const i: I = {a: 1, b: 1, c: 1, d: 1}

The checkers for AB, AC, and D all need to know that {a, b, d} and {a, c, d} are the acceptable sets of properties.

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

1 participant