oozcitak / xmlbuilder-js

An XML builder for node.js
MIT License
918 stars 107 forks source link

Fix typings #212

Closed sandersn closed 5 years ago

sandersn commented 5 years ago
  1. Use correct import syntax for 'stream'
  2. Rename XMLCharacterData -> CharacterData
  3. XMLNode methods return XMLNode instead of XMLElement.

Fixes #211

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 88.469% when pulling 83ff9347039062513a119cc7f138fad69e2a50c2 on sandersn:fix-typings into f16faa2a990de89ad055d8fa857908eac55e0b10 on oozcitak:master.

oozcitak commented 5 years ago

I agree on 1 and 2. For 3, a better resolution would be for XMLDocType to not extend XMLNode at all. In that case some methods and properties from XMLNode need to be redeclared in XMLDocType. Could you please modify your PR accordingly:

Thank you for the PR.

Edit: Also for 2 can you please keep the name XMLCharacterData and make XMLText, XMLComment, etc. extend XMLCharacterData instead of CharacterData. Just to keep names consistent across the library.

sandersn commented 5 years ago

I like the idea of taking XMLDocType out of the inheritance hierarchy. Let me know if that looks right to you.

oozcitak commented 5 years ago

Looks perfect. Thank you. Merging and releasing now.