When updating a nested object's property to 'undefined', the property is not updated

[rbuhrs]

When updating a nested object's property to 'undefined', the property is not updated.

It only gets updated when providing null as property value, or when omitting the property from the object passed to the assign function.

I'm curious if this is intended behavior or a bug?

I created a failing test demonstrating the behavior here: https://github.com/rbuhrs/mikro-orm/blob/d5eefb2d30e3bb29ee122b7858187a0150541733/tests/issues/GH_new_issue.test.ts

[B4nan]

Sounds like a bug in em.assign, does it work when you modify the property directly?

[rbuhrs]

Yes indeed it works when I modify it directly, without the wrap and assign. That is the workaround I was using.

I also added a second test to my fork to demonstrate this: https://github.com/rbuhrs/mikro-orm/blob/d5eefb2d30e3bb29ee122b7858187a0150541733/tests/issues/GH_new_issue.test.ts#L155

[rbuhrs]

@B4nan thanks for fixing 👍

I do have a follow-up question: I noticed that when a nested property is omitted in the data passed to assign, it is not removed.

https://github.com/rbuhrs/mikro-orm/blob/d57c35db8d335ea9e99292a43dbfb93df32c1c6b/tests/features/entity-assigner/GH4428.test.ts#L136
https://github.com/rbuhrs/mikro-orm/blob/d57c35db8d335ea9e99292a43dbfb93df32c1c6b/tests/features/entity-assigner/GH4428.test.ts#L155

I suspect this is by design ? As this behaviour looks the same as in the entity helper docs when using mergeObjects: true (which is the default), and also is how the regular Object.assign function works.

If this is the intended behaviour, would it then be correct to say that:

a) when using mergeObjects, only the props/values which are passed as data are subject to change (including props provided with undefined)

b) when not using mergeObjects, the entity is changed so that it reflects the object passed as data

[B4nan]

Yes

[johanneslumpe]

@B4nan Just flagging that this most likely was a breaking change. Queries like the following used to work fine pre 5.7 and would return the required value if condition was truthy:

repo.findOneOrFail(
        {
          id,
          someProp: condition ?  undefined : { some: value }
        },
)

I believe that fixing this issue caused those queries to now compile down to someProp is null. Not trying to hijack this issue, just posting for posterity that this shouldn't have been included in a minor version bump I believe.

[B4nan]

@johanneslumpe I believe it always behaved like that, we never ignored undefined keys. I can see the very same behavior in v5.6 as well as v5.5.

edit: Ok I guess I know what's happening, you are using filters, right? I can see it behaved like this with filters being applied to the query - but this was rather a bug, not a wanted or documented behaviour, and it worked like that only when filters were in the mix.

edit2: Actually nope, the filters do behave the same, both in v5.7 and earlier versions.

With that said, I am open to discussing whether we should change this behavior in v6 to what you expect (as that is breaking).

[johanneslumpe]

@B4nan Sorry for not getting back to this earlier! I will have to try and reproduce this as I was quite certain that this was breaking. If not, my bad. Definitely open to discussion a potential change! I'll try to get back to you with findings as soon as I can.