Should notes only be of type array? about browser-compat-data HOT 9 CLOSED

It maybe isn't a big enough deal to warrant changing all the files, but it'd always be preferable from the consumer side of things for the JSON to have only one data type if possible.

For my BCD Explorer, I have to write code like this when parsing the notes:

if (jsonKeys.includes('notes')) {
  if (Array.isArray(json.notes)) {
    contentForPopover.set("notes", `Notes: ${json.notes.join("<br>")}`);
  } else {
    contentForPopover.set("notes", `Notes: ${json.notes}`);
  }
}

when I could be writing it something like this:

if (jsonKeys.includes('notes')) {
  contentForPopover.set("notes", `Notes: ${json.notes.join("<br>")}`);
}

from browser-compat-data.

I'm not sure I buy the consistency argument and support_statement is a completely different story to me. Generally, I think having more options will lead to more code, and I think making notes always arrays, would reduce access complexity for at least this property.

from browser-compat-data.

Now that the macro implementation is done for both paths, I don't have any strong feelings anymore either 😆 This would be a good issue for feedback from other consumers, but unfortunately we don't have any (known) users of the data yet.

from browser-compat-data.

We discussed this in Toronto.

I think the version of the schema that we looked at in Toronto had this array-or-string dual form for support_statement but not for notes. There was discussion about whether support_statement should have this dual form. It was decided that it should, then someone said, shouldn't notes have the dual form as well, so as to be consistent.

I agree that having only one form makes the schema and the parsing code both simpler, so that's a good argument in its favour. But having already made the decision one way before, we should be careful we're not overlooking any arguments on the other side. And should we do the same for support_statement?

Note that I'm inconsistent too: the WebExtensions macro assumes that support_statement is a single simple_support_statement object, which works because all the WE data is currently single simple_support_statement. But it shouldn't, really.

from browser-compat-data.

Since we're not putting any code in the notes, supporting multiples statements is a straightforward way to separate notes on different topics.

from browser-compat-data.

FWIW I agree with you, although I don't feel strongly either way.

from browser-compat-data.

Late feedback from my side, though I agree with @Elchi3's initial argument that notes written in plural form is an indicator for only allowing arrays. Looks like that's the only argument that's still valid after the macro implementation now supports both.

Sebastian

from browser-compat-data.

I feel that it should always be an array, for consistency in part, but mainly because it simplifies code paths when interpreting the data, reducing the chances for bugs to occur.

from browser-compat-data.

We've been living with the two paths for a long time now and accept "string or array" for other properties as well, so I think we won't fix this after all.

from browser-compat-data.