Skip to content

fix(mock-doc): move slot event listener support from runtime to MockDoc#6287

Merged
christian-bromann merged 1 commit intomainfrom
cb/move-slot-events-to-mockdoc
Jun 11, 2025
Merged

fix(mock-doc): move slot event listener support from runtime to MockDoc#6287
christian-bromann merged 1 commit intomainfrom
cb/move-slot-events-to-mockdoc

Conversation

@christian-bromann
Copy link
Copy Markdown
Member

What is the current behavior?

The slot event listener fix from PR #6281 was incorrectly implemented in the runtime layer (src/runtime/slot-polyfill-utils.ts and src/runtime/vdom/vdom-render.ts). However, as correctly pointed out by @johnjenkins in #6281 (comment), this issue only manifests during unit testing with Stencil's MockDoc implementation, not in actual runtime scenarios.

The problem occurs because in scoped components, slot elements are polyfilled using text/comment nodes, and MockDoc's MockTextNode class (which inherits from MockNode) lacked the necessary event handling methods (addEventListener, removeEventListener, dispatchEvent), causing "addEventListener is not a function" errors during unit tests.

What is the new behavior?

This PR moves the slot event listener support from the runtime to MockDoc where it properly belongs:

This ensures that all MockDoc node types (including text nodes used for slot polyfills) can handle events during unit testing, while keeping the runtime code clean and focused on actual browser behavior.

Documentation

N/A

Does this introduce a breaking change?

  • Yes
  • No

Testing

  • ✅ Verified all slot onSlotchange tests pass: npm run test.jest -- src/runtime/test/scoped.spec.tsx
  • ✅ Confirmed MockDoc test suite continues to pass: npm run test.jest -- src/mock-doc
  • ✅ Verified runtime tests remain functional: npm run test.jest -- src/runtime/test/render-vdom.spec.tsx
  • ✅ All existing functionality preserved while fixing the architectural issue

Other information

Special thanks to @johnjenkins for correctly identifying that this fix belonged in MockDoc rather than the runtime! 🙏

This refactoring addresses his feedback and results in a cleaner, more appropriate solution that:

  • Keeps runtime code focused on browser behavior
  • Places test-specific fixes in the testing infrastructure (MockDoc)
  • Maintains the same end-user functionality
  • Follows better architectural principles

The original issue from #6269 is still resolved, but now with the fix in the correct layer of the codebase.

Moves the addEventListener/removeEventListener/dispatchEvent support for slot
elements from the runtime to MockDoc where it belongs, since this issue only
affects unit testing with Stencil's mock DOM implementation.

**Root Cause:**
In scoped components, slots are polyfilled using text/comment nodes. During unit
testing, MockDoc's text nodes (MockTextNode) lacked event handling methods,
causing "addEventListener is not a function" errors when attaching slot event
listeners like onSlotchange.

**Solution:**
- Add addEventListener, removeEventListener, and dispatchEvent methods to the
  base MockNode class so all node types can handle events
- Revert the previous runtime changes that incorrectly placed this fix in the
  slot polyfill utilities
- Add appropriate override modifiers to MockElement methods

**Testing:**
- All slot onSlotchange tests pass
- MockDoc test suite continues to pass
- Runtime functionality remains unaffected

Fixes the same issue as the previous runtime fix but places the solution in
the correct layer (MockDoc) rather than in the runtime code.
@christian-bromann christian-bromann added this pull request to the merge queue Jun 11, 2025
@christian-bromann christian-bromann removed this pull request from the merge queue due to a manual request Jun 11, 2025
@christian-bromann christian-bromann merged commit f2dd25d into main Jun 11, 2025
72 checks passed
@christian-bromann christian-bromann deleted the cb/move-slot-events-to-mockdoc branch June 11, 2025 16:33
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