charto / cxml

Advanced schema-aware streaming XML parser
MIT License
45 stars 33 forks source link

Optional Attributes returning wrong Type #6

Open therealmitchconnors opened 7 years ago

therealmitchconnors commented 7 years ago

When an optional attribute is omitted from an xml document, attempting to retrieve the attribute value should return undefined or null, but instead returns an untyped object with _exist: false. This means that null checks against these optional attributes pass, even when they are not specified.

For example, at https://github.com/charto/cxml/blob/master/test/dir-example.ts#L50 , if you were to add: let inode = doc.dir.inode || '' the value of inode would be an object with _exists: false, not a string representing the inode of the directory.

Have I missed some handy feature for detecting unspecified optional attributes?

jjrv commented 7 years ago

You're right, it should return undefined. The problem is probably in information passed to a cleanPlaceholders function. I'll investigate.

jjrv commented 7 years ago

Simply call cxml.init(); and such optional elements / attributes will be removed from the object prototypes. If you call cxml.init(true); then it will even remove placeholders for mandatory content. They will then be undefined.

This should really be documented. However, it's likely that the syntax for removing also mandatory elements / attributes will soon be something like cxml.init({ placeholders: 'none' }).

Note that cxsd produces .d.ts files that assume cxml.init() has been called without arguments. If you add the call to dir-example.ts, running it will currently throw an error on this line:

console.log( dir.file[0]._exists );

The file member is in fact optional according to the schema. If you add "strictNullChecks": true to tsconfig.json, then the compiler or your IDE will also catch that error.

A remaining issue is that the placeholders for strings and numbers should probably never be objects that evaluate as true in a boolean context. null would make more sense.

therealmitchconnors commented 7 years ago

Should init() be called before or after parsing?

jjrv commented 7 years ago

It's called before parsing.

therealmitchconnors commented 7 years ago

That did the trick, thanks! Should this issue be closed as a workaround is available, or left open for tracking of the cleanPlaceholders fix?

jjrv commented 7 years ago

Let's keep it open. I'm doing a major rewrite which should speed up the parsing quite a bit, and will have to keep this issue in mind.