Stricter validation for linking syntax in ReactiveHTML._template (#2689)

* Stricter validation for linking syntax in ReactiveHTML._template

* Add tests

* Update docs

* Clarify child templates

examples/user_guide/Custom_Components.ipynb

@@ -116,15 +116,19 @@
     "A ReactiveHTML component is declared by providing an HTML template on the `_template` attribute on the class. Parameters are synced by inserting them as template variables of the form `${parameter}`, e.g.:\n",
     "\n",
     "```html\n",
-    "    _template = '<div class=\"${div_class}\">${children}</div>'\n",
+    "    _template = '<div id=\"custom_id\" class=\"${div_class}\" onclick=\"${some_method}\">${children}</div>'\n",
     "```\n",
     "\n",
-    "will interpolate the `div_class` parameter on the `class` attribute of the HTML element. In addition to providing attributes we can also provide children to an HTML tag. Any child parameter will be treated as other Panel components to render into the containing HTML. This makes it possible to use `ReactiveHTML` to lay out other components.\n",
+    "will interpolate the `div_class` parameter on the `class` attribute of the HTML element.\n",
     "\n",
-    "The HTML templates also support [Jinja2](https://jinja.palletsprojects.com/en/2.11.x/) syntax to template parameter variables and template child objects. The Jinja2 templating engine is automatically given a few context variables:\n",
+    "In addition to providing attributes we can also provide children to an HTML tag. Any child parameter will be treated as other Panel components to render into the containing HTML. This makes it possible to use `ReactiveHTML` to lay out other components. Lastly the `${}` syntax may also be used to invoke Python methods and JS scripts. Note that you must declare an `id` on components which have linked parameters, methods or children.\n",
+    "\n",
+    "The HTML templates also support [Jinja2](https://jinja.palletsprojects.com/en/2.11.x/) syntax to template parameter variables and child objects. The Jinja2 templating engine is automatically given a few context variables:\n",
     "\n",
     "- `param`: The param namespace object allows templating parameter names, labels, docstrings and other attributes.\n",
-    "- `__doc__`: The class docstring"
+    "- `__doc__`: The class docstring\n",
+    "\n",
+    "The difference between Jinja2 literal templating and the JS templating syntax is important to note. While literal values are inserted during the initial rendering step they are not dynamically linked."
    ]
   },
   {
@@ -136,7 +140,7 @@
     "In order to template other parameters as child objects there are a few options. By default all parameters referenced using `${child}` syntax are treated as if they were Panel components, e.g.:\n",
     "\n",
     "```html\n",
-    "<div id=\"div\">${parameter}</div>\n",
+    "<div id=\"custom_id\">${parameter}</div>\n",
     "```\n",
     "\n",
     "will render the contents of the `parameter` as a Panel object. If you want to render it as a literal string instead you can use the regular Jinja templating syntax instead, i.e. `{{ parameter }}` or you can use the `_child_config` to declare you want to treat `parameter` as a literal:\n",
@@ -165,7 +169,7 @@
     "In certain cases it is necessary to explicitly declare event listeners on the DOM node to ensure that changes in their properties are synced when an event is fired. To make this possible the HTML element in question must be given an `id`, e.g.:\n",
     "\n",
     "```html\n",
-    "    _template = '<input id=\"input\"></input>'\n",
+    "    _template = '<input id=\"custom_id\"></input>'\n",
     "```\n",
     "\n",
     "Now we can use this name to declare set of `_dom_events` to subscribe to. The following will subscribe to change DOM events on the input element:\n",
@@ -174,7 +178,7 @@
     "    _dom_events = {'input': ['change']}\n",
     "```\n",
     "\n",
-    "Once subscribed the class may also define a method following the `_{node}_{event}` naming convention which will fire when the DOM event triggers, e.g. we could define a `_input_change` method. Any such callback will be given a `DOMEvent` object as the first and only argument. The `DOMEvent` contains information about the event on the `.data` attribute and declares the type of event on the `.type` attribute."
+    "Once subscribed the class may also define a method following the `_{node-id}_{event}` naming convention which will fire when the DOM event triggers, e.g. we could define a `__custom_id_change` method. Any such callback will be given a `DOMEvent` object as the first and only argument. The `DOMEvent` contains information about the event on the `.data` attribute and declares the type of event on the `.type` attribute."
    ]
   },
   {
@@ -202,7 +206,7 @@
     "```python\n",
     "    value = param.String()\n",
     "\n",
-    "    _template = '<input id=\"input\" value=\"${value}\"></input>'\n",
+    "    _template = '<input id=\"custom_id\" value=\"${value}\"></input>'\n",
     "```\n",
     "\n",
     "We can now declare a `'value'` key in the `_scripts` dictionary, which will fire whenever the `value` is updated:\n",
@@ -242,15 +246,15 @@
     "Instead of declaring explicit DOM events Python callbacks can also be declared inline, e.g.:\n",
     "\n",
     "```html\n",
-    "    _template = '<input id=\"input\" onchange=\"${_input_change}\"></input>'\n",
+    "    _template = '<input id=\"custom_id\" onchange=\"${_input_change}\"></input>'\n",
     "```\n",
     "\n",
     "will look for an `_input_change` method on the `ReactiveHTML` component and call it when the event is fired.\n",
     "\n",
     "Additionally we can invoke the Javascript code declared in the `_scripts` dictionary by name using the `script` function, e.g.:\n",
     "\n",
     "```html\n",
-    "    <input id=\"input\" onchange=\"${script('some_script')}\"></input>\n",
+    "    <input id=\"custom_id\" onchange=\"${script('some_script')}\"></input>\n",
     "```\n",
     "\n",
     "will invoke the following script if it is defined on the class:\n",
@@ -287,7 +291,7 @@
     "    \n",
     "    index = param.Integer(default=0)\n",
     "    \n",
-    "    _template = '<img id=\"img\" src=\"https://picsum.photos/800/300?image=${index}\" onclick=\"${_img_click}\"></img>'\n",
+    "    _template = '<img id=\"slideshow\" src=\"https://picsum.photos/800/300?image=${index}\" onclick=\"${_img_click}\"></img>'\n",
     "\n",
     "    def _img_click(self, event):\n",
     "        self.index += 1\n",
@@ -312,7 +316,7 @@
     "    \n",
     "    index = param.Integer(default=0)\n",
     "    \n",
-    "    _template = \"\"\"<img id=\"img\" src=\"https://picsum.photos/800/300?image=${index}\" onclick=\"${script('click')}\"></img>\"\"\"\n",
+    "    _template = \"\"\"<img id=\"slideshow\" src=\"https://picsum.photos/800/300?image=${index}\" onclick=\"${script('click')}\"></img>\"\"\"\n",
     "\n",
     "    _scripts = {'click': 'data.index += 1'}\n",
     "    \n",
@@ -354,6 +358,13 @@
     "select"
    ]
   },
+  {
+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "The loop body can declare any number of HTML tags to add for each child object, e.g. to add labels or icons, however the child object (like the `${option}`) must always be wrapped by an HTML element (e.g. `<option>`) which must declare an `id`. Depending on your use case you can wrap each child in any HTML element you require, allowing complex nested components to be declared."
+   ]
+  },
   {
    "cell_type": "markdown",
    "metadata": {},

panel/models/reactive_html.py

@@ -12,9 +12,9 @@
 
 
 endfor = '{% endfor %}'
-list_iter_re = '{% for (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*) %}'
-items_iter_re = '{% for \s*[A-Za-z_]\w*\s*, (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*)\.items\(\) %}'
-values_iter_re = '{% for (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*)\.values\(\) %}'
+list_iter_re = r'{% for (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*) %}'
+items_iter_re = r'{% for \s*[A-Za-z_]\w*\s*, (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*)\.items\(\) %}'
+values_iter_re = r'{% for (\s*[A-Za-z_]\w*\s*) in (\s*[A-Za-z_]\w*\s*)\.values\(\) %}'
 
 
 class ReactiveHTMLParser(HTMLParser):
@@ -27,7 +27,7 @@ def __init__(self, cls, template=True):
         self.children = {}
         self.nodes = []
         self.looped = []
-        self._template_re = re.compile('\$\{[^}]+\}')
+        self._template_re = re.compile(r'\$\{[^}]+\}')
         self._current_node = None
         self._node_stack = []
         self._open_for = False
@@ -40,6 +40,37 @@ def handle_starttag(self, tag, attrs):
         self._node_stack.append((tag, dom_id))
 
         if not dom_id:
+            for attr, value in attrs.items():
+                if value is None:
+                    continue
+                params, methods = [], []
+                for match in self._template_re.findall(value):
+                    match = match[2:-1]
+                    if match.startswith('model.'):
+                        continue
+                    if match in self.cls.param:
+                        params.append(match)
+                    elif hasattr(self.cls, match):
+                        methods.append(match)
+                if methods:
+                    raise ValueError(
+                        "DOM nodes with an attached callback must declare "
+                        f"an id. Found <{tag}> node with the `{attr}` callback "
+                        f"referencing the `{methods[0]}` method. Add an id "
+                        "attribute like this: "
+                        f"<{tag} id=\"{tag}\" {attr}=\"${{{methods[0]}}}>...</{tag}>."
+                    )
+                elif params:
+                    literal = value.replace(f'${{{params[0]}}}', f'{{{{{params[0]}}}}}')
+                    raise ValueError(
+                        "DOM node with a linked parameter declaration "
+                        f"must declare an id. Found <{tag}> node with "
+                        f"the `{attr}` attribute referencing the `{params[0]}` "
+                        "parameter. Either declare an id on the node, "
+                        f"i.e. <{tag} id=\"{tag}\" {attr}=\"{value}\">...</{tag}>, "
+                        "or insert the value as a literal: "
+                        f"<{tag} {attr}=\"{literal}\">...</{tag}>."
+                    )
             return
 
         if dom_id in self.nodes:

panel/tests/test_reactive.py

@@ -2,6 +2,7 @@
 
 import bokeh.core.properties as bp
 import param
+import pytest
 
 from bokeh.models import Div
 from panel.layout import Tabs, WidgetBox
@@ -178,6 +179,26 @@ class Test(ReactiveHTML):
     assert root.callbacks == {}
     assert root.events == {}
 
+def test_reactive_html_no_id_param_error():
+
+    with pytest.raises(ValueError) as excinfo:
+        class Test(ReactiveHTML):
+            width = param.Number(default=200)
+
+            _template = '<div width=${width}></div>'
+
+    assert "Found <div> node with the `width` attribute referencing the `width` parameter." in str(excinfo.value)
+
+def test_reactive_html_no_id_method_error():
+
+    with pytest.raises(ValueError) as excinfo:
+        class Test(ReactiveHTML):
+
+            _template = '<div onclick=${_onclick}></div>'
+
+            def _onclick(self):
+                pass
+    assert "Found <div> node with the `onclick` callback referencing the `_onclick` method." in str(excinfo.value)
 
 def test_reactive_html_dom_events():