dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Cerialize deserializes empty strings as null (no user-visible bug)

Open philipprumpf opened this issue 3 years ago • 2 comments

...at least it does so sometimes (when the fields are tagged with @autoserializeAs(String). As far as I'm aware, this doesn't result in user-visible bugs in DSpace so far, but it's something we've run into using DSpace-CRIS that turned out to be somewhat difficult to debug.

The problem is this code in cerialize.js:

function deserializeString(value) {
    if (Array.isArray(value)) {
        return value.map(function (element) {
            return element && element.toString() || null;
        });
    }
    else {
        return value && value.toString() || null;
    }
}

If value is '', the return value will be null rather than the empty string that was passed in.

As far as I can tell, this only happens for fields decorated with @autoserializeAs(String), not those of type string which are merely decorated with @autoserialize. Changing the latter to the former (I believe this should not change behavior) allows the bug to become observable.

Again, this is not a user-visible bug right now, but it is surprising and arguably incorrect behavior of an underlying library which results in JavaScript objects that violate their TypeScript type descriptions, and it is possible to run into this when extending DSpace.

If people agree this is a cerialize bug, I'll report it there, of course.

philipprumpf avatar Aug 23 '22 11:08 philipprumpf

@philipprumpf : I think this already has a bug ticket in cerialize here: https://github.com/weichx/cerialize/issues/58

Unfortunately, it doesn't appear that cerialize is being maintained. So, it's possible we'd either need to find a way around this in our codebase, or we'd need to see if we can find a different library to replace it.

tdonohue avatar Aug 23 '22 15:08 tdonohue

Oh, thank you! I'd missed that, somehow (or thought it was about the "null" -> null thing). I guess we can't do anything about it for now.

philipprumpf avatar Aug 24 '22 07:08 philipprumpf