Tabcorp/strummer

Strummer v3 proposal

Opened this issue · 6 comments

I have a few changes that I would like to introduce into strummer that will require a major release

  1. match() will return an object { errors: [] }
    • currently is just returns an array of errors
    • by returning an object instead we can then start to return additional things for example with s.integer({ parse: true }) it would be great to return the parsedValue so we are not having to repeat the parsing in our code outside of strummer
  2. s.object will be a strict matcher (basically what s.objectWithOnly is now)
    • s.object currently allow extra fields that are not defined in the schema, so essentially it is acting as a partial object matcher
    • by default I think it would be good to be strict, and if you just want partial matching we can introduce a new s.partial matcher for that purpose, it is more explicit this way
  3. change the params of s.object (and s.partial when it's introduced)
    • right now s.object has a single argument which is the schema itself, the problem with this is that you can't pass in the standard arguments e.g. { optional: true }
    • something like this should be possible s.object({ optional: true, schema: { foo: s.string(), bar: s.number() } })
  1. we could still return array of error objects with extra thing. e.g.
[
  {
    code: '',
    message: '',
    originalValue: '',
    parsedValue: ''
  }
]

this way, we still have the same structure i.e. no breaking change
2. I like this but I am afraid this breaking change could be too much for some people (like me when I blindly upgrade all my libraries to latest version without checking release note). We could still keep s.object and introduce s.partial as alias for this version change, and print out some warning message in the console if people use s.object
3. Yes, but also, could we keep it backward compatible and print out warning message?

@vickvu I think even if we did your suggestion for number 1 it will still be a breaking change, at the moment the presence of a non empty array indicates there are errors, so in the case where there are no errors but there is a parsed value, it would not work, and it's not a very good api

what I'm looking for is the following response

{ errors: [], parsedValue: 123 }

I understand you wanting to keep it backwards compatible but it's been a frustrating thing for a few years now and at some point there will need to be a breaking change

@vickvu @deancouch any more thoughts on this? it's been 6 years since a major release :)

@nguyenchr - I'm fine with having breaking changes, so 👍 for 1)
I'd like to think about 2 and 3 a bit more, get more of an idea about our use cases.

@deancouch @vickvu did you guys have any more thoughts ^^

Any further thoughts on this or is strummer no longer maintained or intending a new version?