dynamodb-toolbox
dynamodb-toolbox copied to clipboard
Updating an attribute to be undefined does not remove the attribute
if you have some item { id: 'asdf', foo: 'asdf'}
and you do Foo.update({id: 'asdf', foo: undefined }
I would expect the field foo to be removed from dynamo but it persists
Isn't this what $remove does?
Ah okay yeah makes sense. Shouldn't the toolbox then throw if (key in payload) { assert(payload[key] !== undefined, 'use $remove')
It depends on your use case I suppose.
Some people see empty/undefined values to have a meaning, so adding the exception carte blanche would be a big opinion.
Well yeah, but if it has meaning then it should do something, otherwise it's an unknown parameter. Anyway, I don't feel that strongly about this, happy for it to be closed. It's just something that tripped me up coming from another dynamo client.
Agreed. I just get the opinion that this library is trying to be as unopinionated as possible.
Perhaps this could be an option to turn on? 'RemoveUndefinedAttributes: true' or similar?
Jumping in late, but I think I agree with @mfbx9da4. There is a removeNullAttributes
option (enabled by default) that will remove attributes set to null
, but I can't remember why I didn't just use undefined
. There was probably a reason, but I'll investigate further.
I think it should probably set to null if null is provided as this a valid dynamo type right? and then remove if set to undefined
NULL is a valid type, but it doesn't make a lot of sense in NoSQL especially if you have a schema defined via the library. I ran a twitter poll on this and "remove the attribute" on nulls won out: https://twitter.com/jeremy_daly/status/1256259584819449856
I'll look more into the undefined thing. I'm guessing I didn't use that because undefined
would get lost on serialization, but that might just be the way the parsing works. I could most likely figure out a workaround.
I don't disagree with removing undefined, however I do note that $remove works just fine as well.
In the context of the DSL for this library, it's more explicit to use the DSL than it is to check for undefined attributes.
That doesn't mean that it's wrong to remove them, but it does introduce another vector for bugs and nuances in debugging that people who use the lib need to be aware of.
I suppose the broader question is to decide which method is the "way" to do it in this library - especially to get it to v1.0.0 rather than a WIP/beta.