NOJS-271: implement sse directive#284
Conversation
NOJS-270 / NOJS-271
New directive `sse` enables declarative real-time data binding via the
browser-native EventSource API. Registered at priority 1 (same tier as
get/post), gated for if-conditional support.
Attribute API: sse (url), as (var name), sse-event (named event),
sse-insert (replace|append|prepend), sse-limit (array cap),
sse-credentials (withCredentials), into (store dual-write),
error (template on terminal close), then (per-message expression).
Reactive $sse state object ({connecting, open, error}) exposed on the
element context via a new switch case in context.js. Per-origin
connection tracking warns at the HTTP/1.1 6-connection limit. Reactive
URL interpolation follows the ancestor-watch pattern from http.js.
All mandatory safety rules enforced: disposal via _onDispose,
el.isConnected guards, watcher cleanup, dispose-before-innerHTML.
Bundle delta: +2,814 bytes (2.75 KB), within 3 KB budget.
ErickXavier
left a comment
There was a problem hiding this comment.
QA Review -- @qa
Verdict: APPROVED -- Clean, spec-compliant implementation of the SSE directive. One should-fix and three nits; none are blockers.
Code Verification
- Quality: Follows project conventions exactly. Code style is indistinguishable from http.js. Imports only what is needed from globals.js, evaluate.js, dom.js, registry.js. No console.log (uses _warn). Private prefix convention followed. ~185 lines, well-structured.
- Security: No new XSS surface -- SSE data is assigned to reactive context variables, never injected as HTML. URL handling goes through _interpolate (safe). Expression evaluation uses _execStatement (CSP-safe, no eval/Function). Store writes use the existing dual-write pattern. Prototype pollution safe via _FORBIDDEN_CTX_KEYS in the proxy set trap. No credential leakage.
- Performance: O(1) replace, O(1) amortized append/prepend with limit. Zero overhead when no sse attribute present.
- Edge cases: See Issues section below.
Safety Rule Compliance (all 12 checked)
- Disposal before DOM clear (Rule 1):
_disposeChildren(el)beforeinnerHTML=""in_renderError(line 117-118). PASS. - Event listener cleanup (Rule 2):
es.close()registered via_onDisposeimmediately after creation (line 146-149). EventSourceclose()kills all listeners per WHATWG spec. PASS (with caveat in S1). - Watcher unsubscribe (Rule 3): All
$watchreturns registered via_onDispose(line 233). Store/route/i18n watcher deletes registered (lines 242, 247, 252). PASS. - Timer guards (Rule 4): No timers;
el.isConnectedguard in all handlers (lines 152, 157, 168). PASS. - Safety-net timeouts (Rule 5): N/A.
- HTML sanitization (Rule 6): No HTML injection of SSE data. PASS.
- Expression identifier resolution (Rule 7): Uses
_execStatement, allow-list only. PASS. - Expression error handling (Rule 8): try-catch around
_execStatementwith_warn(lines 201-207). Better than http.js which has no try-catch on itsthen. PASS. - Cloned elements (Rule 9): Template clone via
_cloneTemplate+processTree. PASS. - Storage persistence (Rule 10): N/A.
- Plugin security (Rule 11): Core directive, frozen after init via
registerDirective. PASS. - Credential handling (Rule 12): N/A (EventSource has no custom headers).
Registration & Wiring
- Priority 1,
gated: true-- matches spec (FR-1). PASS. - Side-effect import in
src/index.jsalongside http.js -- correct pattern. PASS. "sse"added to_RESERVED_GLOBAL_NAMESin index.js. PASS.- context.js: single switch case
$sseafter$form(line 184), matches$formpattern exactly. PASS.
Bundle Delta
- Baseline (feat/NOJS-270): 133,513 bytes
- With SSE: 136,327 bytes
- Delta: +2,814 bytes (2.75 KB) -- within 3 KB budget. PASS.
Existing Test Suite
- 44 suites, 2,387 passed, 0 failed. PASS.
Runtime Verification
- App booted: Yes -- local SSE test server +
dist/iife/no.jsloaded in Chrome. - Feature tested in browser: Yes, via Chrome DevTools MCP.
- Complete flows exercised:
- Replace mode (JSON): ticker data bound and updated correctly (price: 47, symbol: NOJS).
$sse.opendisplayed. PASS. - Append + sse-limit=3: 5 messages sent, array capped at 3 (last 3 kept).
foreachrendered correctly. PASS. - Named events (
sse-event): onlyprice-updateevents bound; default messages ignored. Value: 300. PASS. - Plain text fallback: non-JSON data bound as raw string. Type confirmed as
"string". PASS. - Error template: on terminal close (
readyState === CLOSED), error template rendered ("Connection lost - SSE closed").$sse = {connecting: false, open: false, error: true}. During auto-reconnect, error template NOT displayed (correct). PASS. - then expression:
_execStatementcalled correctly per message. Data binding works. PASS. - 6-connection warning: console shows
[No.JS] SSE: 6 connections to http://localhost:4567...-- correct threshold, correct message. PASS. - $sse state transitions: connecting -> open observed; connecting -> error observed on terminal close. PASS.
- Replace mode (JSON): ticker data bound and updated correctly (price: 47, symbol: NOJS).
- Console errors: None from NoJS. Only expected network errors from deliberately-dropped SSE connections.
Issues Found
S1 (SHOULD-FIX) -- _onDispose inside _openConnection may not register on reactive URL changes -- src/directives/sse.js:146
_onDispose at line 146 captures the local es variable and registers cleanup with _currentEl. During init, _currentEl is set correctly. But when _openConnection is called from onAncestorChange (reactive URL change at line 225), _currentEl may be null or pointing to a different element. In that case, _onDispose silently does nothing (globals.js:279 checks if (_currentEl)), and the new EventSource's close() is never registered for disposal.
Impact: If the element is removed after a URL change but before the next SSE event, the latest EventSource leaks until the el.isConnected guard fires on the next event.
Fix: Add _onDispose(_closeConnection) once during init (~line 88, after defining _closeConnection), when _currentEl is guaranteed to be set. Since _closeConnection references the closure variable _es, it will always close the current connection at disposal time. The _onDispose inside _openConnection can remain as belt-and-suspenders (it works correctly on the initial call).
N1 (NIT) -- No debug logging for SSE lifecycle
http.js uses _log() for debugging. SSE has zero debug output. Adding _log("SSE: connecting to", resolvedUrl) on open and _log("SSE: message received") on message would be consistent and useful.
N2 (NIT) -- Negative/non-numeric sse-limit silently ignored -- src/directives/sse.js:60
sse-limit="-1" or sse-limit="abc" silently bypass limit logic. Consider _warn() for isNaN(limit) || limit < 0.
N3 (NIT) -- PRD/ADR discrepancy on $sse.error during auto-reconnect -- src/directives/sse.js:163
PRD US-7 specifies $sse.error: true when auto-reconnecting. ADR-003 D2 specifies error: false. Implementation follows ADR (correct decision). Recommend documenting the override on the EPIC.
Verdict
APPROVED. S1 is a real but bounded resource-leak risk mitigated by el.isConnected guards. It should be addressed before v1.20.0 release but does not block merging into the feature branch. N1-N3 are quality improvements for follow-up.
What changed
Implements the
ssedirective for declarative Server-Sent Events in NoJS Core, enabling real-time data binding via the browser-native EventSource API with zero JavaScript.Context
Changes
Core directive (
src/directives/sse.js— NEW, ~185 lines)registerDirective("sse", { priority: 1, gated: true, init })— same tier asget/postsse(URL),as(var name, default "data"),sse-event(named event),sse-insert(replace|append|prepend),sse-limit(array cap),sse-credentials(withCredentials),into(store dual-write),error(template on terminal close),then(per-message expression with$event)JSON.parsewith raw-string fallback{expr}interpolation; close+reopen on URL changeMap<origin, Set<EventSource>>; warns at 6 concurrent connections per origin (HTTP/1.1 limit){ connecting, open, error }— connecting on init/reconnect, open on onopen, error only on terminal close (readyState === CLOSED)_onDisposefor EventSource.close(),el.isConnectedguards in all handlers,_disposeChildrenbefore innerHTML swap for error templates, watcher cleanup via_onDisposeContext (
src/context.js)case "$sse": return target.$sse || null;after$formin the$-prefix switchModule entry (
src/index.js)./directives/sse.js(after http.js)"sse"to_RESERVED_GLOBAL_NAMESBuild output (
dist/)Verification evidence
node build.jscleansse-event): only named events processedthenexpression: counter incremented per message$ssestate:show="$sse.connecting"toggles correctlyHow to test
node build.jsnpx jest --no-coveragesse="/endpoint"attributes loadingdist/iife/no.jsChecklist
_prefix,_warn/_logonly, noconsole.log)Files modified
src/directives/sse.js(NEW)src/context.jssrc/index.jsdist/iife/no.jsdist/iife/no.js.map