Add nonStrict modifier for record runtypes

[remnantkevin]

Fixes:

• nonStrict modifier #68
• sloppyRecord looses its sloppyness when modified with partial, pick or omit #23

Description

Context

The default mode of a record runtype is strict, where strictness refers to how a runtype treats excess keys in untrusted input data:

• strict: treats excess keys as an error
• non-strict: ignores excess keys

Because the record runtype is strict by default, excess keys in its input will be treated as an error:

import * as st from "simple-runtypes"

const user = st.record({
  id: st.integer(),
  name: st.string()
})

user({ id: 1, name: "Matt" })
// => { id: 1, name: "Matt" }

user({ id: 1, name: "Matt", isAdmin: true })
// => throws st.RuntypeError: invalid keys in record

st.use(user, { id: 1, name: "Matt", isAdmin: true })
// => { ok: false, error: { reason: 'invalid keys in record [...]', ... } }

simple-runtypes currently also offers a non-strict version of the record runtype, which is called the sloppyRecord runtype. Because it is non-strict, sloppyRecord ignores excess keys:

const sloppyUser = st.sloppyRecord({
  id: st.integer(),
  name: st.string()
})

sloppyUser({ id: 1, name: "Matt" })
// => { id: 1, name: "Matt" }

sloppyUser({ id: 1, name: "Matt", isAdmin: true })
// => { id: 1, name: "Matt" }

st.use(sloppyUser, { id: 1, name: "Matt", isAdmin: true })
// => { ok: true, result: { id: 1, name: "Matt" } }

nonStrict modifier

A non-strict modifier allows strict record runtypes to be changed (modified) into non-strict versions, as opposed to having a separate non-strict runtype (see sloppyRecord above). Some of the reasons why this is desirable are outlined in #68. One of these reasons is that it allows for separation between the record's definition and the way the record is parsed. The modifier approach is similar to the way the pick, omit, and partial runtypes work (they modify a record runtype in order to create a new runtype).

Note: The "sloppy" terminology has negative connotations, and it looks untidy to have the word "sloppy" across a code base. Therefore, "sloppy" has been dropped in favour of "non-strict".

record runtypes continue to be strict by default, and the nonStrict modifier is used on an existing record runtype to create a new, non-strict record runtype. For example:

const nonStrictUser = st.nonStrict(
  st.record({
    id: st.integer(),
    name: st.string()
  })
)

nonStrictUser({ id: 1, name: "Matt" })
// => { id: 1, name: "Matt" }

nonStrictUser({ id: 1, name: "Matt", isAdmin: true })
// => { id: 1, name: "Matt" }

st.use(nonStrictUser, { id: 1, name: "Matt", isAdmin: true })
// => { ok: true, result: { id: 1, name: "Matt" } }

Note: nonStrict does not modify the given runtype in-place, but returns a new runtype based on the given runtype.

Notes on capabilities and limitations

strict and nonStrict can be used in the same runtypes

const user = st.record({
  id: st.integer(),
  name: st.string(),
  company: st.nonStrict(
    st.record({
      id: st.integer(),
      name: st.string()
    })
  )
})

user({ id: 1, name: "Matt", company: { id: 1, name: "EasyJet" } })
// => { id: 1, name: "Matt", company: { id: 1, name: "EasyJet" } }

user({ id: 1, name: "Matt", company: { id: 1, name: "EasyJet", code: "EJ" } })
// => { id: 1, name: "Matt", company: { id: 1, name: "EasyJet" } }

user({ id: 1, name: "Matt", company: { id: 1, name: "EasyJet" }, isAdmin: true })
// => throws st.RuntypeError: invalid keys in record

nonStrict is not recursive

const nonStrictUser = st.nonStrict(
  st.record({
    id: st.integer(),
    name: st.string(),
    address: st.record({
      streetNumber: st.integer(),
      streetName: st.string()
    })
  })
)

nonStrictUser({ id: 1, name: "Matt", address: { streetNumber: 42, streetName: "Sesame St" } })
// => { id: 1, name: "Matt", address: { streetNumber: 42, streetName: "Sesame St" } }

nonStrictUser({ id: 1, name: "Matt", address: { streetNumber: 42, streetName: "Sesame St" } }, isAdmin: true)
// => { id: 1, name: "Matt", address: { streetNumber: 42, streetName: "Sesame St" } }

nonStrictUser({ id: 1, name: "Matt", address: { streetNumber: 42, streetName: "Sesame St", hasAlarm: true } })
// => throws st.RuntypeError: invalid keys in record

nonStrict can only be applied to records

Record runtypes include: record, pick, omit, partial, intersection (of two records).

// ✅
// returns non-strict record runtypes 
st.nonStrict(st.record({ id: st.integer(), name: st.string() }))
st.nonStrict(st.pick(st.record({ id: st.integer(), name: st.string() }), "id"))
st.nonStrict(st.omit(st.record({ id: st.integer(), name: st.string() }), "name"))
st.nonStrict(st.partial(st.record({ id: st.integer(), name: st.string() })))
st.nonStrict(st.intersection(st.record({ name: st.string() }), st.record({ age: st.number() })))

// ❌
// throws st.RuntypeUsageError expected a record runtype
st.nonStrict(st.string())
st.nonStrict(st.array(st.string()))
st.nonStrict(st.union(st.number(), st.string()))
// ...

Compatibility

sloppyRecord can still be used. Under the hood, a sloppyRecord just uses the nonStrict functionality.

[remnantkevin]

@hoeck I have updated the PR description with more details and an overview of the functionality. This is based on my understanding at the moment, but any of it can change based on discussions we have.

I will add my questions and comments about the code itself and the way nonStrict is implemented this weekend, and hopefully we can discuss from there. Look forward to hearing from you about all this 😃

[remnantkevin]

General question about non-strictness, records, and 'combination' runtypes (union, intersection):

Similar to pick, omit, etc., nonStrict relies on the fields property to determine whether the provided runtype is a record runtype. The fields property is needed as its value is used to reconstruct a new, modified runtype from the provided runtype. However, is it the case that all the runtypes that should have fields, do have fields? Are there cases where runtypes that you would expect to have fields (i.e. they seem like they would be record runtypes) don't have fields, either because there is a good reason, or because of a mistake? (I think this might also relate to #89)

I haven't thought through all the possibilities, but I can take a union of records and/or a discriminated union as an example. (But intersection should be considered as well.)

// (A) union of records -> no `fields`
st.union(st.record({ name: st.string() }), st.record({ lastname: st.string() }))

// (B) discriminated union -> `unions`, but no `fields`
st.union(
  st.record({ type: st.literal("standard"), accessLevel: st.number() }),
  st.record({ type: st.literal("pro"), accessLevel: st.number(), admin: st.boolean() })
)

Would we expect to be able to apply pick, omit, nonStrict, etc. to these runtypes? Currently we can't, because both don't have a fields property.

Does this make sense? Does it make sense, for example, to have a union of nonStrict runtypes vs apply nonStrict to a union?

To be clear, at this point, I'm not trying to say what does and does not make sense, but rather just wanting to have the conversation.

(Also consider the various intersection scenarios, and the case in #89)

[hoeck]

Wow! Great work. You've nailed all the important details (nesting, using a symbol for the metadata, ...).

Please merge it if you are happy with my comments.

Things we (I) need to do in order to release this:

• support non-strict in intersections (that runtype looks quite messy to me and maybe needs a little refactoring)
• update the changelog (I need this when I update simple-runtypes at work so I know what has changed between versions)
• update the readme: remove sloppyRecord, add nonStrict documentation

Again, HUGE thanks for the hard and detailed work you've put into this PR!

[remnantkevin]

Sorry for delay on this. Planning on giving it some time this weekend.

[remnantkevin]

TODO

• Add JSDoc comment to nonStrict
• Update README with documentation for nonStrict
• Remove sloppyRecord from README
• Update CHANGELOG -- add to unreleased section
• @hoeck "support non-strict in intersections (that runtype looks quite messy to me and maybe needs a little refactoring)"

[hoeck]

General question about non-strictness, records, and 'combination' runtypes (union, intersection):

...

I think I didn't answer this question? Or did I already?

Would we expect to be able to apply pick, omit, nonStrict, etc. to these runtypes? Currently we can't, because both don't have a fields property.

Does this make sense? Does it make sense, for example, to have a union of nonStrict runtypes vs apply nonStrict to a union?
You made a good point and yes, most things that work in Typescript should work in simple-runtypes (or generally in a good runtype library) too.

Are there cases where runtypes that you would expect to have fields (i.e. they seem like they would be record runtypes) don't have fields, either because there is a good reason, or because of a mistake? (I think this might also relate to #89)

Right, not all records implement the "reflection" interface with .fields and other exposed metadata. That would be a next step to increase the utility of simple-runtypes :D.

#89 was made by coworker while we were translating some untyped Ruby code into Typescript and stumbled across the need to use omit on a non-record runtype.

To be clear, at this point, I'm not trying to say what does and does not make sense, but rather just wanting to have the conversation.

Nah that's fine to suggest stuff, no need to be so defensive I am always open to discuss stuff :D and in that case it completely makes sense.

Given your specific example with unions, it can become quite tricky to exactly mimic Typescripts behavior:

https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgMpjiAJnKXkDeAUMsmAJ4AOEAXMgOQDOG2uW9JyiSjjAMhABuEADZ0QAVwC2AI2hEAvkSKhIsRCgAKUAPaFOFanXqVdHUtwi8Bwsckmz5FrFNB0ZOnSIiZFywygAggg8jMgAvGgsOHjIAD7I2jrKRAD0qWQAFsBhUFYSImBhoFz2EADuyBIgwDogZFQoABSauGDAcCLIOcgARFg5YFDAMhLtwr0AlEQByAAaEYltHSIAPMGhAHzK6Vk9eYwFRd31cPZ1ALTVtfUBM43IAJqLAPKuYOshVowANAyW1iEonomyAA

So it looks like we need to keep union type info in addition to .fields. I you like to we can discuss this in discord or in a new issue.

[remnantkevin]

@hoeck

README

I have updated the README: removing sloppyRecord, adding nonStrict, and various other stylistic and formatting changes. I fully understand that style and formatting are subjective, so feel free to let me know if you'd prefer non/any of the changes to be reverted. Most are an attempt at making the formatting more consistent throughout, and a few are things I noticed a long the way. I changed a few things in the Usage Examples section to try to introduce concepts before they are used, and also to separate out different examples.

Other notes:

1. I can add more detail for nonStrict (e.g. make it 100% clear that it only works on records, and is not recursive). Not sure what info you'd prefer to have in the README vs in JSDoc comments. I could also provide more examples for nonStrict to make these things clear. Or, I could leave it for now and we can come back to that at a later stage.
2. I ran the yarn build-readme. I haven't interacted with this script before in simple-runtypes, so please let me know if anything in the README looks wrong.

CHANGELOG

Next is adding an entry to the CHANGELOG. This would be v7.2.0, right?

Intersections

You mentioned this previously: "support non-strict in intersections (that runtype looks quite messy to me and maybe needs a little refactoring)"

I don't feel like I have the requisite knowledge yet to be able to look into this. So, just checking: will you be looking at this?

[hoeck]

@hoeck

README

I have updated the README: removing sloppyRecord, adding nonStrict, and various other stylistic and formatting changes. I fully understand that style and formatting are subjective, so feel free to let me know if you'd prefer non/any of the changes to be reverted. Most are an attempt at making the formatting more consistent throughout, and a few are things I noticed a long the way. I changed a few things in the Usage Examples section to try to introduce concepts before they are used, and also to separate out different examples.

👍 that looks good thanks!

Other notes:

1. I can add more detail for nonStrict (e.g. make it 100% clear that it only works on records, and is not recursive). Not sure what info you'd prefer to have in the README vs in JSDoc comments. I could also provide more examples for nonStrict to make these things clear. Or, I could leave it for now and we can come back to that at a later stage.

Yes, please leave that for now - in a future version nonStrict should work on any type.

2. I ran the `yarn build-readme`. I haven't interacted with this script before in simple-runtypes, so please let me know if anything in the README looks wrong.

Looks good to me 👍

CHANGELOG

Next is adding an entry to the CHANGELOG. This would be v7.2.0, right?

Right, unless we decide to remove sloppyRecord in the same release then it would be 8.0.0. Or release that together with the record->object rename to not release too many major versions?

But for now you can also use "unreleased" in the changelog. I made the habit of adding all changes immediately to the log as I tend for forget what was all done once I'm ready to release a new version.

Intersections

You mentioned this previously: "support non-strict in intersections (that runtype looks quite messy to me and maybe needs a little refactoring)"

I don't feel like I have the requisite knowledge yet to be able to look into this. So, just checking: will you be looking at this?

Yeah will do that thanks for the reminder!

[remnantkevin]

@hoeck

But for now you can also use "unreleased" in the changelog.

Done 👍

Right, unless we decide to remove sloppyRecord in the same release then it would be 8.0.0. Or release that together with the record->object rename to not release too many major versions?

Maybe?:

• 7.2.0
  • add nonStrict
  • remove sloppyRecord from docs
• 8.0.0
  • remove sloppyRecord
  • rename record to object

I can understand the concern to not release too many major versions, but I also don't think it's worth bundling up too many changes into one release. Obviously up to you at the end of the day though 😄

Yeah will do that thanks for the reminder!

🙂

[hoeck]

Maybe?:

* 7.2.0
  
  * add nonStrict
  * remove sloppyRecord from docs

* 8.0.0
  
  * remove sloppyRecord
  * rename record to object

I can understand the concern to not release too many major versions, but I also don't think it's worth bundling up too many changes into one release. Obviously up to you at the end of the day though smile

Sounds like a good compromise 👍

[remnantkevin]

🎉

@hoeck Thanks for all the guidance and help on this 😄