Fix XML builder and parser to handle null and undefined values#2
Fix XML builder and parser to handle null and undefined values#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the XML builder/parser behavior to better handle “nullish” values and to normalize attribute typing, aligning output with documented expectations.
Changes:
- Builder: serialize
nullas a self-closing element and skipundefinedvalues. - Builder: change empty-string serialization to explicit open/close tags.
- Parser: stop coercing attribute values (always keep them as strings) and drop
xmlnsdeclarations whenremoveNSPrefixis enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/parse.ts | Ensures attribute values remain strings and adjusts namespace-stripping behavior for xmlns declarations. |
| src/build.ts | Adds explicit handling for null (self-closing) and undefined (skipped), and changes empty-string serialization. |
| src/tests/parse.test.ts | Updates expectations to reflect string-only attribute parsing and revised namespace handling. |
| src/tests/build.test.ts | Adds coverage for null/undefined build behavior and updates empty-string expectations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (value === undefined) return; | ||
|
|
||
| if (value === null) { | ||
| const indent = format ? indentBy.repeat(depth) : ''; | ||
| const nl = format ? '\n' : ''; | ||
| parts.push(`${indent}<${key}/>${nl}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
serialize now skips undefined values via an early return, but undefined can still be serialized in other paths: (1) object attributes currently do String(v) and will emit attr="undefined", and (2) object child keys with undefined are still collected into childEntries, which can force the parent into the "has children" branch and produce <parent></parent> instead of a self-closing tag when all children were skipped. Consider filtering out v === undefined when collecting attrParts / childEntries so the "skip undefined values" behavior is consistent and doesn’t affect parent rendering.
| // Primitive value | ||
| const str = String(value); | ||
| if (str === '') { | ||
| parts.push(`${indent}<${key}/>${nl}`); | ||
| } else { | ||
| parts.push(`${indent}<${key}>${escapeXml(str)}</${key}>${nl}`); | ||
| } | ||
| parts.push(`${indent}<${key}>${escapeXml(str)}</${key}>${nl}`); | ||
| } |
There was a problem hiding this comment.
This changes empty-string serialization from a self-closing element (<tag/>) to an explicit open/close pair (<tag></tag>). If this is intentional (e.g., to distinguish '' from null), it should be called out as a behavioral/breaking change in the PR description/release notes; otherwise consider keeping the previous behavior or making it configurable.
| // When stripping namespace prefixes, drop xmlns and xmlns:* declarations entirely | ||
| if (rmNS && (rawName === 'xmlns' || rawName.startsWith('xmlns:'))) continue; |
There was a problem hiding this comment.
With removeNSPrefix enabled, this now drops xmlns / xmlns:* declarations entirely. That’s more than prefix stripping (the option is documented as soap:Body -> Body) and may be breaking for callers that relied on namespace declarations being present in the output attributes. Consider documenting this behavior clearly and/or adding an option to preserve xmlns attributes when needed.
| // When stripping namespace prefixes, drop xmlns and xmlns:* declarations entirely | |
| if (rmNS && (rawName === 'xmlns' || rawName.startsWith('xmlns:'))) continue; | |
| // When stripping namespace prefixes, preserve xmlns and xmlns:* declarations | |
| // so callers can still access namespace information if needed. | |
| if (rmNS && (rawName === 'xmlns' || rawName.startsWith('xmlns:'))) { | |
| if (processEnt) value = decodeEntities(value); | |
| attrs[prefix + rawName] = value; | |
| continue; | |
| } |
Update the XML builder to correctly serialize null as a self-closing tag and skip undefined values. Adjust the parser to ensure attributes are treated as strings, maintaining consistency in handling numeric values.