Fix for anonymousTemplate and nativeTemplateEngine

[phillipc]

Unfortunately, we made two mistakes during the TypeScript porting. Error 1 also masked error 2.

Case 1: nativeTemplateEngine -> The cloneNode function was never used in the native template engine.

Case 2: anonymousTemplate -> The old prototyp construct uses an empty-constructor from domElement, so templateType and element was 'undefined'. I had changed this, so after correcting Case 1, the anonymous templates tests no longer worked.

This all came to light when I started dismantling old IE switches. Here are the fixes.

Summary by CodeRabbit

• Refactor
  • Simplified template processing to remove legacy branching, streamlining how templates are read and parsed for more consistent behavior.
  • Reorganized and tightened template type handling and internal property visibility, improving robustness and reducing risk of template-related bugs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaced IE-specific branching in template node retrieval with a direct call to templateSource.nodes() when available, otherwise falling back to text-based parsing. Added a new templateAnonymous type constant and changed domElement/templateType fields to protected; updated anonymousTemplate typings and behavior.

Changes

Cohort / File(s)	Summary
Template node retrieval simplification packages/binding.template/src/nativeTemplateEngine.ts	Removed IE-version branching and version parsing; now calls templateSource.nodes() if present, otherwise reads templateSource.text() and uses parseHtmlFragment() as before.
Template source types & visibility packages/binding.template/src/templateSources.ts	Added templateAnonymous = 5; changed TemplateSource method overloads (text, nodes) signatures; made domElement.domElement and domElement.templateType protected; anonymousTemplate now sets templateType = templateAnonymous and updated text overload behavior.

Sequence Diagram(s)

(omitted — changes are type/signature and branching simplifications without a multi-component control-flow that benefits from a sequence diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble through code with a curious twitch,
Clearing old branches that used to glitch.
Anonymous types now wear a new hat,
Protected nooks make the structure neat and flat.
Hop, template, hop — cleaner paths at last! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the two main fixes: changes to anonymousTemplate and nativeTemplateEngine, which matches the PR's core objectives.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3422b61 and 007658a.

📒 Files selected for processing (1)

• packages/binding.template/src/templateSources.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: testheadless (24, test-headless-ff)

🔇 Additional comments (4)

packages/binding.template/src/templateSources.ts (4)

33-34: LGTM!

The new templateAnonymous constant follows the established pattern and correctly extends the template type taxonomy to support anonymous templates.

---

36-48: LGTM!

The interface overloads are well-structured. Having explicit signatures for both getter and setter forms improves type inference and IDE support, while the optional-parameter signature provides the implementation contract.

---

153-165: LGTM!

The text() override correctly handles the anonymous template's dual storage mechanism - reading from cached text data or serializing from container nodes when needed. The overload signatures properly mirror the base class.

---

60-61: No breaking change — visibility change is appropriate.

These fields are used internally by the anonymousTemplate subclass and there are no external usages found. Making them protected is the correct visibility level for implementation details accessed by subclasses.

Likely an incorrect or invalid review comment.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 595561d and 3422b61.

📒 Files selected for processing (2)

• packages/binding.template/src/nativeTemplateEngine.ts
• packages/binding.template/src/templateSources.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: testheadless (24, test-headless-ff)

🔇 Additional comments (3)

packages/binding.template/src/templateSources.ts (2)

33-34: LGTM! Anonymous template type properly defined.

The new templateAnonymous constant establishes a distinct template type for anonymous templates, fixing the issue where they previously had undefined templateType values. This aligns with the PR objective of fixing anonymous template behavior.

---

59-60: No issues identified. The fields are internal implementation details with no external accesses detected, making the visibility restriction to protected a safe and beneficial encapsulation improvement.

packages/binding.template/src/nativeTemplateEngine.ts (1)

17-24: LGTM! Template node retrieval now works correctly.

The simplified logic properly fixes Case 1 from the PR objectives by:

1. Using feature detection (templateSource.nodes ? ...) instead of IE version checks
2. Actually calling templateSource.nodes() when available, enabling the cloneNode path (line 20) that was previously unreachable
3. Falling back to text-based parsing when nodes are unavailable (lines 22-23)

This modernization removes legacy IE-specific branching while restoring the intended behavior of preferring DOM nodes over text when available.

[brianmhunt]

Great spotting. It would be good to have tests that verify / prevent a regression, but that doesnt block here. Thank you!