Always render script element as plain HTML (#1239)

* Always render script element as plain HTML

---------

Co-authored-by: James Hutchison <[email protected]>

Author: Archmonger

docs/source/about/changelog.rst

@@ -8,18 +8,21 @@ Changelog
     scheme for the project adheres to `Semantic Versioning <https://semver.org/>`__.
 
 
-.. INSTRUCTIONS FOR CHANGELOG CONTRIBUTORS
-.. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-.. If you're adding a changelog entry, be sure to read the "Creating a Changelog Entry"
-.. section of the documentation before doing so for instructions on how to adhere to the
-.. "Keep a Changelog" style guide (https://keepachangelog.com).
+.. Using the following categories, list your changes in this order:
+.. [Added, Changed, Deprecated, Removed, Fixed, Security]
+.. Don't forget to remove deprecated code on each major release!
 
 Unreleased
 ----------
 
 **Changed**
 
-- :pull:`1251` Substitute client-side usage of ``react`` with ``preact``.
+- :pull:`1251` - Substitute client-side usage of ``react`` with ``preact``.
+- :pull:`1239` - Script elements no longer support behaving like effects. They now strictly behave like plain HTML script elements.
+
+**Fixed**
+
+- :pull:`1239` - Fixed a bug where script elements would not render to the DOM as plain text.
 
 v1.1.0
 ------

pyproject.toml

@@ -54,7 +54,7 @@ artifacts = ["/src/reactpy/static/"]
 artifacts = ["/src/reactpy/static/"]
 
 [tool.hatch.metadata]
-license-files = { paths = ["LICENSE.md"] }
+license-files = { paths = ["LICENSE"] }
 
 [tool.hatch.envs.default]
 installer = "uv"

src/js/packages/@reactpy/client/src/components.tsx

@@ -120,30 +120,33 @@ function ScriptElement({ model }: { model: ReactPyVdom }) {
   const ref = useRef<HTMLDivElement | null>(null);
 
   React.useEffect(() => {
+    // Don't run if the parent element is missing
     if (!ref.current) {
       return;
     }
+
+    // Create the script element
+    const scriptElement: HTMLScriptElement = document.createElement("script");
+    for (const [k, v] of Object.entries(model.attributes || {})) {
+      scriptElement.setAttribute(k, v);
+    }
+
+    // Add the script content as text
     const scriptContent = model?.children?.filter(
       (value): value is string => typeof value == "string",
     )[0];
-
-    let scriptElement: HTMLScriptElement;
-    if (model.attributes) {
-      scriptElement = document.createElement("script");
-      for (const [k, v] of Object.entries(model.attributes)) {
-        scriptElement.setAttribute(k, v);
-      }
-      if (scriptContent) {
-        scriptElement.appendChild(document.createTextNode(scriptContent));
-      }
-      ref.current.appendChild(scriptElement);
-    } else if (scriptContent) {
-      const scriptResult = eval(scriptContent);
-      if (typeof scriptResult == "function") {
-        return scriptResult();
-      }
+    if (scriptContent) {
+      scriptElement.appendChild(document.createTextNode(scriptContent));
     }
-  }, [model.key, ref.current]);
+
+    // Append the script element to the parent element
+    ref.current.appendChild(scriptElement);
+
+    // Remove the script element when the component is unmounted
+    return () => {
+      ref.current?.removeChild(scriptElement);
+    };
+  }, [model.key]);
 
   return <div ref={ref} />;
 }

src/reactpy/html.py

@@ -422,54 +422,10 @@ def _script(
     key is given, the key is inferred to be the content of the script or, lastly its
     'src' attribute if that is given.
 
-    If no attributes are given, the content of the script may evaluate to a function.
-    This function will be called when the script is initially created or when the
-    content of the script changes. The function may itself optionally return a teardown
-    function that is called when the script element is removed from the tree, or when
-    the script content changes.
-
     Notes:
         Do not use unsanitized data from untrusted sources anywhere in your script.
-        Doing so may allow for malicious code injection. Consider this **insecure**
-        code:
-
-        .. code-block::
-
-            my_script = html.script(f"console.log('{user_bio}');")
-
-        A clever attacker could construct ``user_bio`` such that they could escape the
-        string and execute arbitrary code to perform cross-site scripting
-        (`XSS <https://en.wikipedia.org/wiki/Cross-site_scripting>`__`). For example,
-        what if ``user_bio`` were of the form:
-
-        .. code-block:: text
-
-            '); attackerCodeHere(); ('
-
-        This would allow the following Javascript code to be executed client-side:
-
-        .. code-block:: js
-
-            console.log(''); attackerCodeHere(); ('');
-
-        One way to avoid this could be to escape ``user_bio`` so as to prevent the
-        injection of Javascript code. For example:
-
-        .. code-block:: python
-
-            import json
-
-            my_script = html.script(f"console.log({json.dumps(user_bio)});")
-
-        This would prevent the injection of Javascript code by escaping the ``user_bio``
-        string. In this case, the following client-side code would be executed instead:
-
-        .. code-block:: js
-
-            console.log("'); attackerCodeHere(); ('");
-
-        This is a very simple example, but it illustrates the point that you should
-        always be careful when using unsanitized data from untrusted sources.
+        Doing so may allow for malicious code injection
+        (`XSS <https://en.wikipedia.org/wiki/Cross-site_scripting>`__`).
     """
     model: VdomDict = {"tagName": "script"}
 
@@ -481,13 +437,12 @@ def _script(
         if len(children) > 1:
             msg = "'script' nodes may have, at most, one child."
             raise ValueError(msg)
-        elif not isinstance(children[0], str):
+        if not isinstance(children[0], str):
             msg = "The child of a 'script' must be a string."
             raise ValueError(msg)
-        else:
-            model["children"] = children
-            if key is None:
-                key = children[0]
+        model["children"] = children
+        if key is None:
+            key = children[0]
 
     if attributes:
         model["attributes"] = attributes

tests/conftest.py

@@ -36,6 +36,11 @@ def install_playwright():
     subprocess.run(["playwright", "install", "chromium"], check=True)  # noqa: S607, S603
 
 
+@pytest.fixture(autouse=True, scope="session")
+def rebuild_javascript():
+    subprocess.run(["hatch", "run", "javascript:build"], check=True)  # noqa: S607, S603
+
+
 @pytest.fixture
 async def display(server, page):
     async with DisplayFixture(server, page) as display:

tests/test_html.py

@@ -3,47 +3,7 @@
 from reactpy import component, config, html
 from reactpy.testing import DisplayFixture, poll
 from reactpy.utils import Ref
-from tests.tooling.hooks import use_counter, use_toggle
-
-
-async def test_script_mount_unmount(display: DisplayFixture):
-    toggle_is_mounted = Ref()
-
-    @component
-    def Root():
-        is_mounted, toggle_is_mounted.current = use_toggle(True)
-        return html.div(
-            html.div({"id": "mount-state", "data_value": False}),
-            HasScript() if is_mounted else html.div(),
-        )
-
-    @component
-    def HasScript():
-        return html.script(
-            """() => {
-                const mapping = {"false": false, "true": true};
-                const mountStateEl = document.getElementById("mount-state");
-                mountStateEl.setAttribute(
-                    "data-value", !mapping[mountStateEl.getAttribute("data-value")]);
-                return () => mountStateEl.setAttribute(
-                    "data-value", !mapping[mountStateEl.getAttribute("data-value")]);
-            }"""
-        )
-
-    await display.show(Root)
-
-    mount_state = await display.page.wait_for_selector("#mount-state", state="attached")
-    poll_mount_state = poll(mount_state.get_attribute, "data-value")
-
-    await poll_mount_state.until_equals("true")
-
-    toggle_is_mounted.current()
-
-    await poll_mount_state.until_equals("false")
-
-    toggle_is_mounted.current()
-
-    await poll_mount_state.until_equals("true")
+from tests.tooling.hooks import use_counter
 
 
 async def test_script_re_run_on_content_change(display: DisplayFixture):
@@ -54,14 +14,8 @@ def HasScript():
         count, incr_count.current = use_counter(1)
         return html.div(
             html.div({"id": "mount-count", "data_value": 0}),
-            html.div({"id": "unmount-count", "data_value": 0}),
             html.script(
-                f"""() => {{
-                    const mountCountEl = document.getElementById("mount-count");
-                    const unmountCountEl = document.getElementById("unmount-count");
-                    mountCountEl.setAttribute("data-value", {count});
-                    return () => unmountCountEl.setAttribute("data-value", {count});;
-                }}"""
+                f'document.getElementById("mount-count").setAttribute("data-value", {count});'
             ),
         )
 
@@ -70,23 +24,11 @@ def HasScript():
     mount_count = await display.page.wait_for_selector("#mount-count", state="attached")
     poll_mount_count = poll(mount_count.get_attribute, "data-value")
 
-    unmount_count = await display.page.wait_for_selector(
-        "#unmount-count", state="attached"
-    )
-    poll_unmount_count = poll(unmount_count.get_attribute, "data-value")
-
     await poll_mount_count.until_equals("1")
-    await poll_unmount_count.until_equals("0")
-
     incr_count.current()
-
     await poll_mount_count.until_equals("2")
-    await poll_unmount_count.until_equals("1")
-
     incr_count.current()
-
     await poll_mount_count.until_equals("3")
-    await poll_unmount_count.until_equals("2")
 
 
 async def test_script_from_src(display: DisplayFixture):

tests/test_web/test_module.py

@@ -50,6 +50,7 @@ def ShowCurrentComponent():
     await display.page.wait_for_selector("#unmount-flag", state="attached")
 
 
+@pytest.mark.flaky(reruns=3)
 async def test_module_from_url(browser):
     app = Sanic("test_module_from_url")
 

tests/tooling/common.py

@@ -1,9 +1,12 @@
+import os
 from typing import Any
 
 from reactpy.core.types import LayoutEventMessage, LayoutUpdateMessage
 
-# see: https://github.com/microsoft/playwright-python/issues/1614
-DEFAULT_TYPE_DELAY = 100  # milliseconds
+GITHUB_ACTIONS = os.getenv("GITHUB_ACTIONS", "False")
+DEFAULT_TYPE_DELAY = (
+    250 if GITHUB_ACTIONS.lower() in {"y", "yes", "t", "true", "on", "1"} else 25
+)
 
 
 def event_message(target: str, *data: Any) -> LayoutEventMessage: