Skip to content

fix: accept null/empty-string in fragments and harden test coverage - #130

Merged
voxpelli merged 3 commits into
mainfrom
fix/html-template-value-type
Mar 1, 2026
Merged

voxpelli merged 3 commits into
mainfrom
fix/html-template-value-type

Conversation

@voxpelli

@voxpelli voxpelli commented Mar 1, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • html tagged template correctly normalizes falsy interpolation values (null, undefined, false) to '' via _checkHtmlResult, but multi-root fragments kept those empty strings in the returned array
  • render() rejects '' via its own falsy guard (if (!item) throw), causing html${element}${null}${null}`` to throw at render time
  • Fix: filter '' from the array output in html() so only meaningful renderable values reach the render paths — fixes both sync and async paths automatically
  • Type-narrow render()/renderSync() to accept empty strings as valid HtmlMethodResult

Code review follow-ups

  • Align /* c8 ignore next */ comment style with -- exhaustiveness guard annotation across render.js
  • Replace weak assert.equal(result.length, 1) with assert.deepEqual(result, [element]) in array filtering test
  • Add Promise.resolve(null) fragment test covering the async path (the stated PR motivation)
  • Add renderToStringSync('') test mirroring the async render('') test
  • Add symbol rejection type test for HtmlTemplateValue

Test plan

  • End-to-end async tests for null, false, undefined, and Promise.resolve(null) fragment interpolations
  • End-to-end sync tests for null, false, undefined, and empty string fragment interpolations
  • Array shape test verifying empty strings are filtered with content assertion
  • false and symbol type assertions for HtmlTemplateValue (typetests/index.test.ts)
  • Full test gauntlet passes: tsc, eslint, knip, type-coverage ≥99%, tstyche (TS 5.8 + 5.9), 196 runtime tests

🤖 Generated with Claude Code

When html`${element}${null}${null}` produces a multi-root array,
_checkHtmlResult normalizes falsy values to '', but render() rejects
empty strings via its own falsy guard. Filter '' from the array output
so only meaningful renderable values reach the render paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@voxpelli
voxpelli force-pushed the fix/html-template-value-type branch from b5ce41a to 408c83b Compare March 1, 2026 21:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a rendering bug where multi-root html``...`` results could contain '' entries (from null/undefined/false interpolations), which then caused render() / renderToStringSync() to throw due to their non-falsy guards.

Changes:

  • Filter '' values out of multi-root array results returned by html() to prevent render-time failures.
  • Introduce/extend template typing via HtmlTemplateValue and add related type-level regression coverage.
  • Add runtime tests covering fragment interpolations for null/false/undefined in both async and sync render paths.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/htm.js Filters empty-string entries from multi-root html() arrays; updates value typing for the internal htm binding.
lib/element-types.d.ts Adds HtmlTemplateValue and adjusts ElementPropsValue Set/Map typing.
test/render-complex.spec.js Adds async end-to-end tests for fragment interpolations (null/false/undefined) and removes now-unnecessary TS ignores.
test/sync-render.spec.js Adds sync end-to-end tests for fragment interpolations (null/false).
test/html.spec.js Adds an array-shape test asserting '' values are filtered from multi-root results; tweaks TS ignore comment.
test/async-render.spec.js Removes TS ignore comments now covered by updated typing.
typetests/index.test.ts Adds type regression tests for HtmlTemplateValue composition and Set/Map assignability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/htm.js
Replace blunt `if (!item)` falsy check with proper typeof/Array.isArray
narrowing and assertTypeIsNever exhaustiveness guard. This fixes
Promise-resolved empty strings (e.g. `Promise.resolve('')`) being
incorrectly rejected — the Promise survives html()'s sync .filter()
but after await produces '' which the old falsy check threw on.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/render.js
Align c8 ignore comment style, strengthen test assertions,
and add missing test coverage for edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@voxpelli voxpelli changed the title fix: filter empty strings from multi-root fragment arrays fix: accept null/empty-string in fragments and harden test coverage Mar 1, 2026
@voxpelli
voxpelli merged commit 17f8afb into main Mar 1, 2026
16 of 17 checks passed
@voxpelli
voxpelli deleted the fix/html-template-value-type branch March 1, 2026 22:13
@vp-helper vp-helper Bot mentioned this pull request Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants