Skip to content
This repository has been archived by the owner on Dec 19, 2022. It is now read-only.

Strummer v3 proposal #191

Open
nguyenchr opened this issue Jun 2, 2021 · 6 comments
Open

Strummer v3 proposal #191

nguyenchr opened this issue Jun 2, 2021 · 6 comments
Assignees

Comments

@nguyenchr
Copy link
Contributor

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() } })
@vickvu
Copy link

vickvu commented Jun 3, 2021

  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?

@nguyenchr
Copy link
Contributor Author

nguyenchr commented Jun 3, 2021

@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

@nguyenchr
Copy link
Contributor Author

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

@deancouch
Copy link
Contributor

@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.

@nguyenchr
Copy link
Contributor Author

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

@alec-pirillo-le
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants