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

Add a flag to check/validate/guard which allows enforcing exact match (not just structurally assignable) #41

Open
artparks opened this issue Feb 21, 2018 · 15 comments · May be fixed by #162

Comments

@artparks
Copy link

Perhaps I'm missing something fundamental here about how this library makes assertions, but the following obviously errors in Typescript:

interface ISeries {
  name: string;
  value: number;
}

const series: ISeries = {
  name: 'test',
  value: 10,
  foo: true,
};

but this check() does not error with runtypes:

const Series = Record({
  name: String,
  value: Number,
});

Series.check({
  name: 'test',
  value: 10,
  foo: true
});

Why is the additional key foo allowed through in this case?

@pelotom
Copy link
Collaborator

pelotom commented Feb 21, 2018

Your example is only an error when assigning an object literal with extra properties. In the general case, TypeScript's structural typing says that extra fields on an object are completely permissible. You can see this by making the following small modification:

interface ISeries {
  name: string;
  value: number;
}

// foo: { name: string; value: number; foo: boolean; }
const foo = {
  name: 'test',
  value: 10,
  foo: true,
};

const series: ISeries = foo; // no error

The check in question only applies to object literals, not variables, because this is thought to be an indication of improper use; why would you declare properties on an object literal which will be immediately forgotten by the type of the variable they are being assigned to? However in the general case it is a feature, not a bug, of structural typing to allow extra properties, because then a single object is structurally usable in many different scenarios.

The type system is able to make this distinction and do a stricter check specifically in the case of object literal assignment, because it has complete information about your program. In our case, we have no idea whether what is being checked is an object literal or a variable, so we opt for the general structural typing rule.

That said, I would consider adding an optional strict flag to check() which would allow the user to choose to make this kind of check for extraneous "own" properties.

@artparks
Copy link
Author

Thanks for the reply. So my TS example was rather contrived, and I agree, not particularly useful. But what I was hoping to use this library for was just a simple validation that a data structure, created dynamically by the user at runtime, exactly matches a specified type before allowing it into the application. In my case, were that object to have an extraneous property, it would cause problems if allowed in. It does feel like it would be a useful option bearing in mind the kind of use cases I imagine this library fulfils?

@pelotom
Copy link
Collaborator

pelotom commented Feb 21, 2018

Yep, it does sound useful.

@breathe
Copy link
Contributor

breathe commented Feb 21, 2018

I would find such a strict option interesting — but for the use I have in mind it would need to be able retrieve all the extraneous property failures in a single call to check ...

I’d use it to implement a “mask” method to create a deep copy of an object with only the properties defined via a particular runtype. I sometimes find the need to do operations like this when dealing with various JavaScript sdk’s/rest endpoints that validate to ensure that only expected properties are present (aws-sdk).

I haven’t found a super great general pattern to correct for this — especially when using typescript classes with private properties for data modeling.

@pelotom pelotom changed the title additional fields in Records don't throw errors Add a flag to check/validate/guard which allows enforcing exact match (not just structurally assignable) Feb 21, 2018
@Eoksni
Copy link

Eoksni commented Jul 21, 2019

Any progress on this? I would love to have this feature, even willing to PR it (if you guys point me in the right direction on implementing it).

My use-case is runtime checks of type definitions for Javascript library. So I have JS library, I write .d.ts for it and now I want to add some tests to assure that my .d.ts is up-to-date with the real js. And it makes sense if it fails when someone introduces new public function/property in the js.

@pelotom
Copy link
Collaborator

pelotom commented Jul 23, 2019

A PR would be welcome.

@schibrikov
Copy link
Contributor

schibrikov commented Dec 26, 2019

@pelotom
Probably I'd like to work on this one.
Should it finally look like Asteroid.check(obj, { strict: true }) ?
So we are adding options object to check signature here. Should it be added to all Runtypes or only Record?

@pelotom
Copy link
Collaborator

pelotom commented Dec 28, 2019

@pelotom
Probably I'd like to work on this one.
Should it finally look like Asteroid.check(obj, { strict: true }) ?
So we are adding options object to check signature here. Should it be added to all Runtypes or only Record?

Let’s call the flag exact; this matches the name of the corresponding proposed typescript feature (“exact types”). My feeling is it should be added to the general check method and passed down recursively, so that if you are checking a union of records for example, each of the records will be checked exactly.

@zakzag
Copy link

zakzag commented May 11, 2020

@pelotom
Probably I'd like to work on this one.
Should it finally look like Asteroid.check(obj, { strict: true }) ?
So we are adding options object to check signature here. Should it be added to all Runtypes or only Record?

Let’s call the flag exact; this matches the name of the corresponding proposed typescript feature (“exact types”). My feeling is it should be added to the general check method and passed down recursively, so that if you are checking a union of records for example, each of the records will be checked exactly.

This feature would be extremely helpful for me. Any progress?

@yuhr
Copy link
Collaborator

yuhr commented Apr 2, 2021

I'm now thinking that Records should do exact validation by default (of course this is a breaking change), in favor of the risk described in #111 i.e. the value returned from Record(...).check(...) may contain sensitive properties potentially that don't appear on its static type.

This change should come along with #201 so that you can allow unknown keys, but unfortunately there upstream exists several disappointing limitations described in microsoft/TypeScript#7765 and microsoft/TypeScript#17867, which prevents us from having arbitrary number of unknown properties with proper typing in a generic way.

So, the ideal API would look:

Record({ ..., [Rest(String)]: Unknown })

While a temporary workaround would look like:

Record({ ... }).withRest(Unknown, String)

@bayareacoder
Copy link

bayareacoder commented Apr 2, 2021

I agree Record should be an exact match AND allow for KNOWN optional keys, to address issues explained here:
#160 (comment)

@milankinen
Copy link

milankinen commented Jun 1, 2021

If we want to preserve the backwards compatibility, how about if we add configuration options to .check and .validate that allow called to define which behaviour should be used?

So basically something like this:

type Config = {
  extraKeys: 'allow' | 'drop' | 'strict' 
}

const defaults: Config = {
  extraKeys: 'allow'
}

interface Runtype<A> {
  assert(value: unknown, config?: Config): asserts value is A
  check(value: unknown, config?: Config): A
  validate(value: unknown, config?: Config): Result
  guard(value: unknown, config?: Config): value is A
}

If extraKeys is set to allow (which is default for backwards compatibility), .check and .validate behave like they've behaved before, returning the original object. If it's set to drop (or strip or similar), extra keys won't cause any errors but they're removed from the output. strict doesn't allow extra keys at all, failing in such cases. This config approach would also enable more extensions in future.

To be honest since assert and guard are mainly used as TS type guards and because TS uses structural typing, I'm not sure that the config is relevant for those functions at all... 🤔

That said, I also see some potential use cases for explicitly defining exact objects that'd fail validations regardless of the extraKeys config. For those, I really like the current Optional higher order approach. Perhaps we could exact schemas with the same way?

const Planet = Record({
  type: Literal('planet'),
  location: Vector,
  mass: Number,
  population: Number,
  habitable: Boolean,
});

const ExactPlanet = Exact(Planet) // or Strict(Planet)
// or 
const ExactPlanet = Planet.exact() // or Planet.strict()

That, however, raises a question: should "exact" modifier apply recursively to the nested types as well? My intuition says that it should but not entirely sure.

@jupiter
Copy link

jupiter commented Sep 21, 2021

Here's a snippet that demonstrates how @milankinen's suggestion can work with a simple Constraint applied to a Record, Partial or Intersection of those.

To achieve this:

const ExactPlanet = Exact(Planet)

ExactPlanet.check({ ..., alien: 1 }) // throws "Failed Exact(record) check"

// or
const ExactExtendedPlanet = Exact(Planet.And(Partial({ optionalExtendedField: Boolean }))

ExactExtendedPlanet.check({ ..., alien: 1 }) // throws "Failed Exact(intersection) check"

Use this:

import { Runtype } from "runtypes";

const isRuntypeWithAlternatives = (x: Runtype): x is Runtype & { alternatives: Runtype[] } => "alternatives" in x;
const isRuntypeWithIntersectees = (x: Runtype): x is Runtype & { intersectees: Runtype[] } => "intersectees" in x;
const isRuntypeWithFields = (
  x: Runtype,
): x is Runtype & {
  fields: {
    [_: string]: Runtype;
  };
} => "fields" in x;

const fieldsFromRuntype = (runtype: Runtype): string[] => {
  if (isRuntypeWithFields(runtype)) return Object.keys(runtype.fields);
  if (isRuntypeWithIntersectees(runtype))
    return runtype.intersectees.flatMap(intersectee => fieldsFromRuntype(intersectee));
  return [];
};

const isObject = (x: unknown): x is object => typeof x === "object" && x !== null;

export function Exact<TRuntype extends Runtype>(runtype: TRuntype): TRuntype {
  if (isRuntypeWithAlternatives(runtype)) throw Error("Exact cannot be applied to a Union, use Exact on each member of the Union");
  const fields = fieldsFromRuntype(runtype);
  return <TRuntype>(
    (<unknown>(
      runtype.withConstraint(x => isObject(x) && !Object.keys(x).some(key => !fields.includes(key)), {
        name: `Exact(${runtype.reflect.tag})`,
      })
    ))
  );
}

For Union / Or types, you need to apply Exact to every member though, e.g.

const ExactBody = ExactPlanet.Or(Exact(Star))

// or

const ExactBody = Union(ExactPlanet, Exact(Star))

To @milankinen's question of whether to apply recursively: I also thought initially that it should apply recursively and it worked for Partial / Record / Intersection, but with Union it gets tricky.

@yuhr With a Union + Exact on every member, you can actually achieve an XOR behaviour.

@kael-shipman
Copy link

Hey folks! Love this library and am very interested in this feature. Any progress on this? I see it's been a while since there's been an update.

@DavidHanover
Copy link

also interested in this feature

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

Successfully merging a pull request may close this issue.