From e887c0d97836c9df0b8e1ff89b6e42f2fbfed80e Mon Sep 17 00:00:00 2001
From: giles <giles.bradshaw@rose-ash.com>
Date: Tue, 24 Mar 2026 15:48:19 +0000
Subject: [PATCH] =?UTF-8?q?Fix=20defisland=E2=86=92Component=20bug=20in=20?=
 =?UTF-8?q?jinja=5Fbridge;=20add=20island=20reactivity=20test?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

jinja_bridge.py was creating Component objects for both defcomp AND
defisland forms. Islands need Island objects so the serializer emits
defisland (not defcomp) in the client component bundle. Without this,
client-side islands don't get data-sx-island attributes, hydration
fails, and all reactive signals (colour cycling, stepper) stop working.

Add Playwright test: islands hydrate, stepper buttons update count,
reactive colour cycling works on click.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 shared/sx/jinja_bridge.py           | 42 ++++++++++++++++++++++++++---
 tests/playwright/isomorphic.spec.js | 22 +++++++++++++++
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/shared/sx/jinja_bridge.py b/shared/sx/jinja_bridge.py
index cb51e1d..e70fba4 100644
--- a/shared/sx/jinja_bridge.py
+++ b/shared/sx/jinja_bridge.py
@@ -396,6 +396,40 @@ def load_handler_dir(directory: str, service_name: str) -> None:
         _load(directory, service_name)
 
 
+def _parse_defcomp_params(param_form: list) -> tuple[list[str], bool]:
+    """Extract keyword param names and has_children from a defcomp param list.
+
+    Handles: (&key p1 p2 &rest children), (&key (p1 :as type) &rest children),
+    (p1 p2), () etc.
+
+    Returns (param_names, has_children).
+    """
+    if not isinstance(param_form, list):
+        return [], False
+
+    params: list[str] = []
+    has_children = False
+    in_key = False
+    i = 0
+    while i < len(param_form):
+        item = param_form[i]
+        if isinstance(item, Symbol):
+            sname = item.name
+            if sname == "&key":
+                in_key = True
+            elif sname == "&rest":
+                has_children = True
+                i += 1  # skip the rest-param name (e.g. 'children')
+            else:
+                params.append(sname)
+        elif isinstance(item, list):
+            # Typed param: (name :as type)
+            if item and isinstance(item[0], Symbol):
+                params.append(item[0].name)
+        i += 1
+    return params, has_children
+
+
 def register_components(sx_source: str, *, _defer_postprocess: bool = False) -> None:
     """Parse and evaluate s-expression component definitions into the
     shared environment.
@@ -410,7 +444,7 @@ def register_components(sx_source: str, *, _defer_postprocess: bool = False) ->
     existing = set(_COMPONENT_ENV.keys())
 
     # Evaluate definitions — OCaml kernel handles everything.
-    # Python-side component registry is populated minimally for CSS/deps.
+    # Python-side component registry is populated with parsed params for CSS/deps.
     exprs = parse_all(sx_source)
     for expr in exprs:
         if (isinstance(expr, list) and expr and isinstance(expr[0], Symbol)
@@ -420,9 +454,11 @@ def register_components(sx_source: str, *, _defer_postprocess: bool = False) ->
             name_sym = expr[1] if len(expr) > 1 else None
             name = name_sym.name if hasattr(name_sym, 'name') else str(name_sym) if name_sym else None
             if name and expr[0].name in ("defcomp", "defisland"):
-                _COMPONENT_ENV[name] = Component(
+                params, has_children = _parse_defcomp_params(expr[2] if len(expr) > 3 else [])
+                cls = Island if expr[0].name == "defisland" else Component
+                _COMPONENT_ENV[name] = cls(
                     name=name.lstrip("~"),
-                    params=[], has_children=False,
+                    params=params, has_children=has_children,
                     body=expr[-1], closure={},
                 )
 
diff --git a/tests/playwright/isomorphic.spec.js b/tests/playwright/isomorphic.spec.js
index a1839f0..3729d68 100644
--- a/tests/playwright/isomorphic.spec.js
+++ b/tests/playwright/isomorphic.spec.js
@@ -136,6 +136,28 @@ test.describe('Isomorphic SSR', () => {
     await context.close();
   });
 
+  test('islands hydrate and reactive signals work', async ({ page }) => {
+    await page.goto(BASE_URL + '/sx/', { waitUntil: 'networkidle' });
+    await page.waitForSelector('[data-sx-island="layouts/header"]', { timeout: 15000 });
+    await page.waitForSelector('[data-sx-island="home/stepper"]', { timeout: 5000 });
+
+    // Stepper buttons change the count
+    const stepper = page.locator('[data-sx-island="home/stepper"]');
+    const textBefore = await stepper.textContent();
+    await stepper.locator('button').last().click();
+    await page.waitForTimeout(300);
+    const textAfter = await stepper.textContent();
+    expect(textAfter).not.toBe(textBefore);
+
+    // Reactive colour cycling on "reactive" word
+    const reactive = page.locator('[data-sx-island="layouts/header"]').getByText('reactive');
+    const colourBefore = await reactive.evaluate(el => el.style.color);
+    await reactive.click();
+    await page.waitForTimeout(300);
+    const colourAfter = await reactive.evaluate(el => el.style.color);
+    expect(colourAfter).not.toBe(colourBefore);
+  });
+
   test('navigation links have valid URLs (no [object Object])', async ({ page }) => {
     await page.goto(BASE_URL + '/sx/', { waitUntil: 'networkidle' });
     await page.waitForTimeout(1000);