hoeck / simple-runtypes

Small, efficient and extendable runtype library for Typescript
MIT License
114 stars 5 forks source link

`sloppyRecord` in `array` doesn't remove extra properties #60

Closed remnantkevin closed 2 years ago

remnantkevin commented 2 years ago

In the following example I would have expected the age property (eventInput.attendees[0].age) to be removed, and so not be present in event.result, because it is not specified in the attendees array of SloppyEvent (it is an extra property). However, the age property is present in the result.

const SloppyEvent = st.sloppyRecord({
  title: st.string(),
  attendees: st.array(st.sloppyRecord({ name: st.string() }))
})
const eventInput = { title: "Superbowl", attendees: [{ name: "Joe", age: 5 }] };
const event = st.use(SloppyEvent, eventInput);
assert.deepEqual(event, { ok: true, result: eventInput }); // ✗ 'attendees[0].age' property not removed

See online reproduction here (on replit.com).

I'm new to simple-runtypes so please ignore if I've misunderstood how the above should work.

hoeck commented 2 years ago

:open_mouth: oh shit you're totally right! As soon as you use sloppyRecord as part of another container runtype it might stop removing extra properties.

Good catch and thanks :heart: for looking so deeply into the record details.

Would you like to fix it yourself and provide a PR or do you want me to fix it?

If you like, I can help you finding the issue and navigating the code. Some tests for other runtypes probably need to be extended too to guard against the same kind of bug.

remnantkevin commented 2 years ago

I'd be happy/keen to try fix it 🙂 I'm just not sure I have the required knowledge/skills in TS and other things to do so. However, if you're open helping me, and for me to give it a go, that sounds good 👍

hoeck commented 2 years ago

Great, I'm happy to help / mentor :smile:.

Guess the first step would be checking out the repo, install all dependencies, get the tests running and read the code at record.ts to understand how things are supposed to work.

After understanding the bug, I think it would be a good idea to fix it in a test-driven style: write a test (or a few) that fails and then implement the bug fix making the test go green.

But I don't know what the best format for communication is. Maybe we keep discussing stuff it here but that might take me some time to answer. Or we find some time where we're both online and move to Discord (or any other chat app)? Any thoughts?

remnantkevin commented 2 years ago

Sorry for delay in responding, work has just been a bit hectic as of late.

Thank you for the suggestions and willingness to help 😄

I've used Discord before so that sounds good 👍 (unless you'd prefer something else)

I'm planning to start looking at this this weekend 👍

hoeck commented 2 years ago

Sorry for delay in responding, work has just been a bit hectic as of late.

No hurries, I am quite busy too, this weekend there is a small local barcamp I plan to attend.

I've created a simple-runtypes discord server invite link

I'm planning to start looking at this this weekend +1

:+1: ask me anything on the discord and I'll try to respond in time.