RFC: More general type checking for structural typings

Hi all,

We are evaluating a feature to allow a common pattern for configurations, the idea is to allow such things type check:

type config = {
   "x" :  int,
   "y" : option < int>, 
   "z" : option < int>,
   ... // more optionals
}
let fn  = (c : config) => {
  ...
}
fn( {"x": 3 }) // This currently fails to compile

The github issue is here, let us know if you come across such nuances, and if you have other similar use cases, thanks!

20 Likes

What would this compile to? All fields defined and set to undefined, or megamorphic objects?

it just type checks, no code instrumented since this is still sound

All I can think of is that this proposal fails to encode the difference between { x: 3 } and { x: 3, y: undefined}, but why would anyone in their right minds make y: undefined have different semantics than a lack of y?

1 Like

For example, if using the in operator in JS (suddenly a lot more common than it was 5 years ago due to typescript), it makes a difference.

Just to clarify, this means that the following values would all be compatible types?

let a = {"a": 1}
let b = {"a": 1, "b": None}
let c = {"a": 1, "b": Some(2)}

I’ve definitely used JS libraries that treat those differently. They just check if a property exists without checking its actual value, so setting a property to undefined would cause runtime crashes. IIRC there are some built-in browser/DOM APIs that do that too (although I could be misremembering).

But I don’t think that’s really a mark against this proposal. In the end, you’re still dealing with external bindings, and those have always been unsafe and full of gotchas. This proposal seems like it would just make it easier to deal with those cases when they come up.

This would be really great for a lot of things. Not just config objects, but also e.g. React Native style objects. :slight_smile:

2 Likes

This is awesome, in my opinion that was the last remaining pain point when writing externals!

6 Likes

When we discuss type soundness, it is in rescript world. In JS world, everything is observable. So if you find a case where you can write type checked code and it has type error under this proposal, would be great to hear

I’ve definitely used JS libraries that treat those differently. They just check if a property exists without checking its actual value, so setting a property to undefined would cause runtime crashes.

Can you elaborate a little bit, is it purely in the rescript world?

@cknitt @tsnobip
Glad that you find it useful, it would be great to have your use cases elaborated so we can make sure our effort will cover your use cases.

1 Like

Not in ReScript, no. One JS library off the top of my head is LocalForage. It’s uses a config object and just checks if the object properties exist or not. Setting the fields to None/undefined crashes it. I’ve recently bound to it in ReScript with @obj for optional fields. This proposal would definitely make it more convenient though.

Would a similar thing also be considered for records?

What would be completely amazing if the syntax between structurally typed objects and records could be unified, and that whether a record is structural or nominal can be maybe set with an annotation on the type? Probably I it is super hard to implement this, but that would be a dream scenario.

Anyway this change would be pretty great for a lot of things when binding to JS. Especially a lot of API’s have a configuration object with a lot of optional values, having a creator function with optional arguments creates a lot of boilerplate code, especially if the objects are nested (which they often are). So I like this proposal!

4 Likes

To elaborate: This is very useful when authoring bindings and binding to JS object types with many optional fields. This is usually the case for config objects as you described. Another use case would be React Native style objects.

In JS, such styles (contained in a StyleSheet object) look like this:

const styles = StyleSheet.create({
  text: {
    textAlign: "center",
    marginTop: 64.0,
    padding: 20.0
  },
  screen: {
    flex: 1.0,
    backgroundColor: "white"
  },
});

In ReScript, we currently cannot have records or structural objects with optional fields (that can just be left out instead of set to None).

So in the current ReScript React Native bindings, we need to model style objects using an abstract type and @obj external ... if we want zero-cost bindings. This way, the ReScript equivalent to the above JS code currently looks like this:

open Style

let styles =
  StyleSheet.create({
    "text":
      style(
        ~textAlign=`center,
        ~marginTop=dp(64.0),
        ~padding=dp(20.0),
        (),
      ),
    "screen": style(~flex=1.0, ~backgroundColor="white", ()),
  })

Ideally (though that would be a major breaking change for the bindings) we would want the style definitions to be as close to the JS version as possible (without having to resort to a PPX solution).

E.g.

let styles =
  StyleSheet.create({
    "text": {
      "textAlign": `center,
      "marginTop": dp(64.0),
      "padding": dp(20.0)
    },
    "screen": {
      "flex": 1.0,
      "backgroundColor": "white"
    },
  })
1 Like

Would a similar thing also be considered for records?

That would make the performance model a bit hard to reason about (V8 IC), we will see how this proposal goes first.

having a creator function with optional arguments creates a lot of boilerplate code,

This is inspired from that, I saw some functions which has hundreds of arguments, this is clearly not what we want.

1 Like

Is this because the shape of the object changes when you leave out the undefined values? I think just having the shorter syntax would already be a big win. Maybe compressing the undefined values out could be a separate feature, or available just on objects?

it’s mostly when bindings to JS libs that use option objects.

For example today I’m binding this with:

  type t

  @obj
  external make:
    (
      ~ownerConnectionString: string=?,
      ~retryOnInitFail: bool=?,
      ~watchPg: bool=?,
      ~pgDefaultRole: string=?,
      ~dynamicJson: bool=?,
      ~setofFunctionsContainNulls: bool=?,
      ~classicIds: bool=?,
      ~disableDefaultMutations: bool=?,
      ~ignoreRBAC: bool=?,
      ~ignoreIndexes: bool=?,
      ~includeExtensionResources: bool=?,
      // ~showErrorStack: bool or 'json',
      // ~extendedErrors: any combo of ['hint', 'detail', 'errcode'],
      ~handleErrors: array(GraphQlError.t) => array(GraphQlError.t)=?,
      ~appendPlugins: array(GraphileBuild.plugin)=?,
      ~prependPlugins: array(GraphileBuild.plugin)=?,
      ~replaceAllPlugins: array(GraphileBuild.plugin)=?,
      ~skipPlugins: array(GraphileBuild.plugin)=?,
      ~readCache: string=?,
      ~writeCache: string=?,
      ~exportJsonSchemaPath: string=?,
      ~exportGqlSchemaPath: string=?,
      ~sortExport: bool=?,
      ~graphqlRoute: string=?,
      ~graphiqlRoute: string=?,
      ~externalUrlBase: string=?,
      ~graphiql: bool=?,
      ~enhanceGraphiql: bool=?,
      ~enableCors: bool=?,
      ~bodySizeLimit: string=?, // human readable e.g. 200kb or 5MB
      ~enableQueryBatching: bool=?,
      ~jwtSecret: string=?,
      ~jwtVerifyOptions: JWTVerifyOptions.t=?,
      ~jwtRole: string=?,
      // ~jwtAudiences: string=?, // deprecated
      ~jwtPgTypeIdentifier: string=?,
      ~legacyRelations: string=?, // only, deprecated, or omit
      ~legacyJsonUuid: bool=?,
      ~disableQueryLog: bool=?,
      ~pgSettings: Express.Request.t => Js.Dict.t(string)=?,
      ~additionalGraphQLContextFromRequest: Express.Request.t =>
                                            Js.Dict.t(string)
                                              =?,
      ~pluginHook: pluginHookOutput=?,
      ~simpleCollections: [ | `omit | `only | `both]=?,
      ~queryCacheMaxSize: int=?,
      ~simpleSubscriptions: bool=?,
      ~subscriptions: bool=?,
      ~subscriptionAuthorizationFunction: string=?,
      ~readOnlyConnection: string=?,
      ~defaultPaginationCap: int=?,
      ~graphqlDepthLimit: int=?,
      ~graphqlCostLimit: int=?,
      ~exposeGraphQLCost: bool=?,
      ~graphileBuildOptions: GraphileBuildOptions.t=?,
      ~live: bool=?,
      ~allowExplain: unit => bool=?,
      unit
    ) =>
    t
}

Being able to just use a simple rescript object would definitely be nicer, especially when you have nested config objects.

2 Likes

This sounds great!
Our backend (Node) relies heavily on Pulumi an AWS SDK (js libs). Both usually use config objects with lots of optional properties.
Currently we use @obj everywhere. This feature could simplify our bindings and usage.

1 Like

I think for the objects using structural typing this would be a good feature to have – but I think as others have pointed out there is definitely a desire for the ability to specify Exact types like in flow. TypeScript has had a feature request for exact types for 4+ years now https://github.com/microsoft/TypeScript/issues/12936#issue-195716702

Personally, I like the confidence that records always have the runtime representation that they’re defined with. Maybe this is an edge case for others but I have quite often run into runtime problems with TypeScript where the structural typing fails in conjunction with GraphQL’s runtime type checking – we have mutation inputs that are similar in shape to query results for our forms, and if the __typename property is not deleted from the mutation input (form values being initialized from query results) we get a runtime error. I like being able to point at nominal typing with records as an example of ReScript giving us additional type safety.

2 Likes

I think exactness as defined in TypeScript or Flow is about limiting additional fields on an object, while this proposal is about allowing fewer fields on an object. ReScript has exactness check on objects today and this proposal would not affect it. Both Flow and TS have a concept of optional fields where leaving them off or setting as undefined is valid. They are related but different enough that I think it’s worth mentioning. Here’s a small example of both.

2 Likes