Skip to content

BREAKING(xml): expose doctype and processing instructions from parse - #7261

Open
tomas-zijdemans wants to merge 3 commits into
denoland:mainfrom
tomas-zijdemans:xml-expose-doctype-pi
Open

BREAKING(xml): expose doctype and processing instructions from parse#7261
tomas-zijdemans wants to merge 3 commits into
denoland:mainfrom
tomas-zijdemans:xml-expose-doctype-pi

Conversation

@tomas-zijdemans

Copy link
Copy Markdown
Contributor

parse() validated processing instructions and the DOCTYPE, then threw both away. This PR keeps them: the doctype lands on XmlDocument.doctype, and PIs become tree nodes. This addresses the gap noted at the end of #7212.

XML 1.0 §2.6 is blunt about PIs: they "MUST be passed through to the application". Comments outside the root element were dropped the same way, so a license header above <root> vanished on round-trip. Also fixed.

What changed

  • New XmlDoctype type, exposed as XmlDocument.doctype. Captures the name, public id, and system id. Only produced with disallowDoctype: false. The secure default still rejects DOCTYPE outright.
  • New XmlProcessingInstructionNode in the XmlNode union, plus an isProcessingInstruction guard. PIs inside the root land in children.
  • PIs and comments outside the root land in new prolog/epilog fields on XmlDocument, in document order, omitted when empty.
  • ignoreProcessingInstructions moved from ParseStreamOptions to BaseParseOptions so the sync parser accepts it too. Streaming behavior is unchanged.
  • stringify() emits all of the above and gains a doctype: boolean option (default true), mirroring declaration.

Breaking changes

  • XmlNode gains a member: exhaustive switches over node types need a processing_instruction case.
  • PIs now appear in children by default. Opt out with ignoreProcessingInstructions: true.
  • A DOCTYPE after the root element, or a second DOCTYPE, now throws XmlSyntaxError (XML 1.0 §2.8). Both were silently accepted before, which becomes observable once the doctype is stored. Only reachable with disallowDoctype: false.

Fidelity notes

  • The DTD internal subset is validated but never captured (DTD content is deliberately unprocessed), so parse → stringify drops it.
  • Doctype/PI interleaving is not preserved: a PI written before <!DOCTYPE> re-serializes after it. Still valid per §2.8.
  • <!DOCTYPE foo PUBLIC "pub"> (public id, no system id) is leniently parsed and re-serialized as-is, matching the DOM XMLSerializer.

Testing

46 new tests across parser, serializer, and type guards, including round-trips: XHTML doctype, xml-stylesheet PI, internal-subset drop. One existing test asserted the old discarding behavior and was rewritten. deno task ok passes.

The two commits split cleanly: doctype first, PIs second, in case one half needs to land alone.

Built with assistance from Claude Code

tomas-zijdemans and others added 2 commits July 31, 2026 12:42
The sync parser walked the whole DOCTYPE declaration but stored nothing.
Capture the name and PUBLIC/SYSTEM identifiers into a new XmlDoctype type
exposed as XmlDocument.doctype (only reachable with disallowDoctype: false),
and teach stringify to emit it behind a new doctype option (default true).

Breaking: a DOCTYPE after the root element or a second DOCTYPE now throws
(XML 1.0 §2.8); both were silently accepted before, which becomes observable
now that the doctype is stored.

The DTD internal subset is deliberately not captured and is dropped on
round-trip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sync parser validated processing instructions and then discarded them,
even though XML 1.0 §2.6 requires PIs to be passed through to the
application. Comments outside the root element were silently dropped the
same way.

Add XmlProcessingInstructionNode to the XmlNode union, collect PIs and
comments outside the root into new prolog/epilog fields on XmlDocument
(omitted when empty), and teach stringify to emit all of it. Move the
ignoreProcessingInstructions option from ParseStreamOptions to
BaseParseOptions so the sync parser accepts it too; streaming behavior is
unchanged. Add the isProcessingInstruction type guard.

Breaking: exhaustive switches over XmlNode gain a case, and PIs now appear
in children by default for documents that contain them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the xml label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.41379% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.03%. Comparing base (cdb83c6) to head (27efa94).

Files with missing lines Patch % Lines
xml/stringify.ts 95.52% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7261    +/-   ##
========================================
  Coverage   95.03%   95.03%            
========================================
  Files         618      618            
  Lines       51596    51954   +358     
  Branches     9340     9434    +94     
========================================
+ Hits        49035    49376   +341     
- Misses       2021     2032    +11     
- Partials      540      546     +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant