[API DISCUSSION] Handling multiple values for text frames

[Zazama]

This is more of a discussion thread, it would be cool to get some input of users 🙂

ID3v2.4.0 defines that text frames can have multiple values (seperated by 0x00). This is of course problematic with the current implementation, because right now, users get an Object and they expect the value to be of type string.

const tags = NodeID3.read(...)
// error if tags.text is Array!
tags.artist.split('something')

1. In order to fix this, we could always return an Array, but I'm not sure how elegant that is

const tags = NodeID3.read(...)
if(tags.artist) {
    tags.artist[0].split('something')
}

2. There could be a switch between string and Array depending on the value, but it would also not be very clean

const tags = NodeID3.read(...)
if(tags.artist && tags.artist instanceof Array) {
    tags.artist[0].split('something')
} else {
    tags.artist.split('something')
}

3. We could implement a new API for returned tags

const tags = NodeID3.read(...)
// returns undefined/string/Array
tags.artist().value()
// returns undefined/string
tags.artist().first()
// returns Array
tags.artist().all()
// number (basically tags.artist().all().length)
tags.artist().size()

tags.get('TPE1').value()
tags.get('TPE1').first()
tags.get('TPE1').all()
tags.get('TPE1').size()

And add an option to return a normal object instead

const tags = NodeID3.read(..., { returnObject: true })
tags.artist // can have type undefined/string/Array

Or even something like

const tags = NodeID3.read(..., { returnObject: true, splitChar: '$' })
tags.artist // always string, multiple values split by $

[jin60641]

how about always return the string(joined) and add optional parameter for returning always array for text frames? you said, because right now, users get an Object and they expect the value to be of type string. so, keeping string type is good way to reduce side effects for current users.

const tags1 = NodeID3.read(..., { splitTextFrame: true }) // default false
tags1.artist // will be ['hello', 'hello2'] 
const tags2 = NodeID3.read(...)
tags2.artist // will be 'hello hello2'. no side effects for current users.

[kbuffington]

When I handled this in my fork, I went with the hybrid approach and had it return either a string or array in the case of multiple values. My main reason for doing it this way was on the node-id3 side of things because then it was blatantly obvious whether we were looking at a single or multiValue text field, which made writing in particular a lot simpler.

Anyway, the hybrid approach was fairly easy to handle in my consumer, since internally I was just converting everything into a string because my UI was just manipulating strings:

if (Array.isArray(value)) {
        value = value.join('; ');
}

And then if I needed to set a value back out to node-id3, I'd just do a .split(';') and send the array out back out.

[pbricout]

This is quite tricky, because this new requirement introduced in version 4, is kind of backward compatible on read but not on write. None of the various solutions discussed so far are good work around and all come with issues. The most important point is that it is not possible to handle the tags with multiple text values safely without changing the code of the existing clients. For example, joining the strings with a separator has multiple issues:

• how to know which character is safe to join on?
• we can't safely default to a character, but even if we do, if the client modify the string there is a risk a breaking it
• even once joined the client still need to be aware of this, modifying the string without knowing this will potentially break it

So I think we should not try to be backward compatible and introduce a breaking change. Trying to work around will introduce more issues than benefits, Code will be unsafe and more complicated both for the library and the client, Let's say the library is not in version 1.0 and is consequently still in unstable mode. I would introduce a new minor version and specify that a breaking change has been introduced to force clients to handle correctly potential multiple values. I would always use an array in create and read.