-
Notifications
You must be signed in to change notification settings - Fork 34
Implementing json-to-xml() support in XSLT.
#147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success576 tests passing in 44 suites. Report generated by 🧪jest coverage report action from 4958378 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds json-to-xml() support to the XSLT/XPath execution path (XSLT 3.0) and introduces tests/docs updates to validate and document the new capability.
Changes:
- Implements
json-to-xml()as a custom XPath function and converts its produced nodes into the library’sXNodemodel. - Adds a comprehensive new test suite for
json-to-xml()behavior. - Updates README wording/structure and refreshes copyright headers.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/xslt/xslt.test.tsx | Copyright header update. |
| tests/xslt/xslt-validation.test.tsx | Copyright header update. |
| tests/xml/xmltoken.test.tsx | Copyright header update. |
| tests/dom.test.tsx | Copyright header update. |
| src/xslt/xslt.ts | Copyright header update. |
| src/xpath/xpath.ts | Adds json-to-xml() function support and XPathNode→XNode conversion logic; passes xsltVersion into context. |
| src/xpath/tokens.ts | Copyright header update. |
| src/xpath/match-resolver.ts | Copyright header update. |
| src/xpath/lib | Updates XPath subproject commit to include needed functionality. |
| src/xpath/index.ts | Copyright header update. |
| src/dom/xmltoken.ts | Copyright header update. |
| src/dom/util.ts | Copyright header update. |
| src/dom/functions.ts | Copyright header update. |
| tests/json-to-xml.test.tsx | New test suite covering json-to-xml() conversion scenarios and error handling. |
| README.md | Fixes typo, adds note about XPath repo, changes unpkg URL, and relocates breaking-changes section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // json-to-xml() function - XSLT 3.0 specific | ||
| functions['json-to-xml'] = (jsonText: any) => { | ||
| // Check XSLT version - only supported in 3.0 | ||
| if (exprContext.xsltVersion === '1.0') { | ||
| throw new Error('json-to-xml() is not supported in XSLT 1.0. Use version="3.0" in your stylesheet.'); | ||
| } |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json-to-xml() is documented/implemented as XSLT 3.0-specific, but the guard only rejects XSLT 1.0. As written, XSLT 2.0 (or other versions) would incorrectly allow the function. Update the version check to allow only XSLT 3.0 (or explicitly reject anything other than 3.0) and adjust the error message accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
| if (!jsonStr) { | ||
| return null; | ||
| } |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For XPath/XSLT functions that conceptually return a node-set/sequence, returning null can be ambiguous and can force downstream callers/operators to special-case it. Consider returning an empty node-set representation (e.g., []) for empty/invalid inputs to behave more like an empty sequence in XPath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@leonelsanchesdasilva I've opened a new pull request, #150, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@leonelsanchesdasilva I've opened a new pull request, #151, to work on those changes. Once the pull request is ready, I'll request review from you. |
Resolves #136.