Properties as Array

[Alcionei]

In the old version, the XML properties they were not being treated as an array. With the new version behavior changed all properties are now array.

I think the previous behavior was correct.

Old version:
if (_this.options.mergeAttrs) {
obj[processedKey] = newValue;
}
New version change:
if (_this.options.mergeAttrs) {
_this.assignOrPush(obj, processedKey, newValue);
}

[Laro88]

<meter METER_ID="00523614" INTERFACE="02">
METER_ID And INTERFACE becomes an arrays, but why this design decision?

[Leonidas-from-XIV]

@Laro88 What METER_ID and INTERFACE are you talking about?

[Laro88]

I was thinking while writing, xml sample provided now:
METER_ID is parsed correctly here (as a property):

<?xml version="1.0" encoding="utf-8"?> 
<interface MESSAGE_TYPE="1"> 
<muc MUC_ID="967a3" VERSION="1f4" TIMESTAMP="1465815796"> 
<meter METER_ID="00523614" INTERFACE="02"> 
<data DESCRIPTION="Meter status byte" UNIT="Bin" SCALE="1" MEDIUM="Heat (inlet)" OBIS_ID="FF"> 
<entry> 
<param NAME="T">1465700340</param> 
<param NAME="T_MUC">1465700400</param> 
<param NAME="VAL">144</param> 
</entry> 
</data> 
....

Here METER_ID is an array which is erroneous:

<?xml version="1.0" encoding="utf-8"?> 
<interface MESSAGE_TYPE="1"> 
<muc MUC_ID="987fe" VERSION="1f4" TIMESTAMP="1465815992"> 
<meter METER_ID="12345678" INTERFACE="05"> 
<data DESCRIPTION="Meter status byte" UNIT="Bin" SCALE="1" MEDIUM="Heat (inlet)" OBIS_ID="FF"> 
<entry> 
<param NAME="T">1465418580</param> 
<param NAME="T_MUC">1465506596</param> 
<param NAME="VAL">48</param> 
</entry> 
</data> 
...

I have cut of the full xml, it is valid, full files available upon request.

The data is coming from a "concentrator" grabbing date from some wireless sensors and relaying them to my Node.JS code. I found this issue when adding mergeAttrs : true - it would make the code more readable, but I am hitting this issue with properties becoming arrays.

[Laro88]

I cant reproduce the behavior in a unit test. I will fault back to the $ and _ semantics just to be safe.

[gaurav21r]

I have this issue too. It happens when mergeAttrs: true

[gaurav21r]

#309 Addresses this!

[jcsahnwaldt]

I think this issue should be closed as "Won't fix".

It makes sense to have attribute values in arrays when mergeAttr: true and explicitArray: true is set. This behavior was introduced in PR #101.

Even with mergeAttr: true and explicitArray: false, an attribute value has to be put into an array if there is an element with the same name as the attribute.

Example:

const xml2js = require('xml2js');
let parser = new xml2js.Parser({
  mergeAttrs: true,
  explicitArray: false,
});
let xml = `<x y='attVal'><y>eleVal</y></x>`;
parser.parseString(xml, (err, res) => {
  console.log(res);
});

Output:

{ x: { y: [ 'attVal', 'eleVal' ] } }

[IRCraziestTaxi]

@jcsahnwaldt In what way does that make sense? If I have a node with attributes on it:

<Node
    attr1 = "val1"
    attr2 = "val2"
>
</Node>

I expect the JSON representation to have those properties as primitive values and not an array with one element.

With nodes, yes, explicitArray: true does make sense because you cannot safely assume that just because only one instance of a certain type of node exists that there could not be more of them.

However, with attributes, they are just strings. Why should those be parsed as an array containing only that value?

[jcsahnwaldt]

@IRCraziestTaxi

<foo bar="bar"><bar>baz</bar></foo>

See #101 for details.

[IRCraziestTaxi]

@jcsahnwaldt Although that is a valid point in the wild west world of XML, is that really a realistic schema? Why would there ever be an attribute and a child node named the exact same thing? I believe we should if nothing else have an additional option to disregard attributes when combining explicitArray: true and mergeAttrs: true.

I am simultaneously replying to your comment on #309 for more context for anybody who may come across this.

[jcsahnwaldt]

I'll reply at #309.

[IRCraziestTaxi]

@jcsahnwaldt I put up a PR with a new option to allow users to fix this: #516. Please take a look at it and let me know what you think.

[IRCraziestTaxi]

@jcsahnwaldt Did you get a chance to review the PR? We could use this change in our project, so I'd like to either merge the changes sooner than later or find out what should be done differently.

[jcsahnwaldt]

@IRCraziestTaxi I'm sorry, I didn't have time yet, and I probably won't have time for another few weeks or so. Also, I'm not a committer on this project. I created a few PRs about a year ago, but I haven't worked with node-xml2js since. It looks like @Leonidas-from-XIV doesn't have much time to look into PRs either (I'm not blaming him). I guess your best option may be to create a fork, implement the changes you need, and use your own (patched) version of node-xml2js.

[IRCraziestTaxi]

Update:

For anyone coming across this issue and wanting the fix, while we're waiting for my PR to be reviewed, place this in the dependencies section of your package.json in order to use my fork with the fix for this problem:

    "xml2js": "https://github.com/IRCraziestTaxi/node-xml2js/tarball/304-properties-as-array-fix"

Usage:

    xmlParseOptions: {
        ignoreAttrs: false,
        mergeAttrs: true,
        mergeAttrsArray: false
    }

[Leonidas-from-XIV]

I am in full agreement with @jcsahnwaldt here.