add option to unmountBeforeUpdate

this is less than ideal - as such #461 has been created
in order to try and track down what the problem is. in
short victory charts do not update unless explicitely
unmounted before updating. ReactDOM.render should work
as expected, but for whatever reason it is not here.

Author: rmorshea

docs/source/examples/simple_dashboard.py

@@ -6,7 +6,12 @@
 from idom.widgets import Input
 
 
-victory = idom.web.module_from_template("react", "victory", fallback="⌛")
+victory = idom.web.module_from_template(
+    "react",
+    "victory-line",
+    fallback="loading...",
+    unmount_before_update=True,
+)
 VictoryLine = idom.web.export(victory, "VictoryLine")
 
 
@@ -77,8 +82,8 @@ def update_value(value):
         set_value_callback(value)
 
     return idom.html.fieldset(
-        {"className": "number-input-container"},
-        [idom.html.legend({"style": {"fontSize": "medium"}}, label)],
+        {"class": "number-input-container"},
+        [idom.html.legend({"style": {"font-size": "medium"}}, label)],
         Input(update_value, "number", value, attributes=attrs, cast=float),
         Input(update_value, "range", value, attributes=attrs, cast=float),
     )

docs/source/examples/victory_chart.py

@@ -1,7 +1,7 @@
 import idom
 
 
-victory = idom.web.module_from_template("react", "victory", fallback="⌛")
+victory = idom.web.module_from_template("react", "victory-line", fallback="⌛")
 VictoryBar = idom.web.export(victory, "VictoryBar")
 
 bar_style = {"parent": {"width": "500px"}, "data": {"fill": "royalblue"}}

src/client/packages/idom-client-react/src/component.js

@@ -72,20 +72,25 @@ function ImportedElement({ model }) {
   }
 }
 
-function RenderImportedElement() {
+function RenderImportedElement({ model, importSource }) {
   const config = React.useContext(LayoutConfigContext);
   const mountPoint = React.useRef(null);
   const sourceBinding = React.useRef(null);
 
   React.useEffect(() => {
     sourceBinding.current = importSource.bind(mountPoint.current);
-    return () => {
-      sourceBinding.current.unmount();
-    };
+    if (!importSource.data.unmountBeforeUpdate) {
+      return sourceBinding.current.unmount;
+    }
   }, []);
 
   // this effect must run every time in case the model has changed
-  React.useEffect(() => sourceBinding.current.render(model));
+  React.useEffect(() => {
+    sourceBinding.current.render(model);
+    if (importSource.data.unmountBeforeUpdate) {
+      return sourceBinding.current.unmount;
+    }
+  });
 
   return html`<div ref=${mountPoint} />`;
 }
@@ -145,6 +150,7 @@ function loadImportSource(config, importSource) {
     .then((module) => {
       if (typeof module.bind == "function") {
         return {
+          data: importSource,
           bind: (node) => {
             const binding = module.bind(node, config);
             if (

src/idom/core/vdom.py

@@ -77,6 +77,7 @@
                     "if": {"not": {"type": "null"}},
                     "then": {"$ref": "#/definitions/elementOrString"},
                 },
+                "unmountBeforeUpdate": {"type": "boolean"},
             },
             "required": ["source"],
         },
@@ -289,6 +290,7 @@ class ImportSourceDict(TypedDict):
     source: str
     fallback: Any
     sourceType: str  # noqa
+    unmountBeforeUpdate: bool  # noqa
 
 
 class _OptionalVdomJson(TypedDict, total=False):

src/idom/web/module.py

@@ -36,6 +36,7 @@ def module_from_url(
     fallback: Optional[Any] = None,
     resolve_exports: bool = IDOM_DEBUG_MODE.current,
     resolve_exports_depth: int = 5,
+    unmount_before_update: bool = False,
 ) -> WebModule:
     """Load a :class:`WebModule` from a :data:`URL_SOURCE`
 
@@ -49,6 +50,11 @@ def module_from_url(
             Whether to try and find all the named exports of this module.
         resolve_exports_depth:
             How deeply to search for those exports.
+        unmount_before_update:
+            Cause the component to be unmounted before each update. This option should
+            only be used if the imported package failes to re-render when props change.
+            Using this option has negative performance consequences since all DOM
+            elements must be changed on each render. See :issue:`461` for more info.
     """
     return WebModule(
         source=url,
@@ -60,6 +66,7 @@ def module_from_url(
             if resolve_exports
             else None
         ),
+        unmount_before_update=unmount_before_update,
     )
 
 
@@ -70,6 +77,7 @@ def module_from_template(
     fallback: Optional[Any] = None,
     resolve_exports: bool = IDOM_DEBUG_MODE.current,
     resolve_exports_depth: int = 5,
+    unmount_before_update: bool = False,
 ) -> WebModule:
     """Create a :class:`WebModule` from a framework template
 
@@ -98,6 +106,11 @@ def module_from_template(
             Whether to try and find all the named exports of this module.
         resolve_exports_depth:
             How deeply to search for those exports.
+        unmount_before_update:
+            Cause the component to be unmounted before each update. This option should
+            only be used if the imported package failes to re-render when props change.
+            Using this option has negative performance consequences since all DOM
+            elements must be changed on each render. See :issue:`461` for more info.
     """
     # We do this since the package may be any valid URL path. Thus we may need to strip
     # object parameters or query information so we save the resulting template under the
@@ -129,6 +142,7 @@ def module_from_template(
             if resolve_exports
             else None
         ),
+        unmount_before_update=unmount_before_update,
     )
 
 
@@ -139,6 +153,7 @@ def module_from_file(
     resolve_exports: bool = IDOM_DEBUG_MODE.current,
     resolve_exports_depth: int = 5,
     symlink: bool = False,
+    unmount_before_update: bool = False,
 ) -> WebModule:
     """Load a :class:`WebModule` from a :data:`URL_SOURCE` using a known framework
 
@@ -156,6 +171,11 @@ def module_from_file(
             Whether to try and find all the named exports of this module.
         resolve_exports_depth:
             How deeply to search for those exports.
+        unmount_before_update:
+            Cause the component to be unmounted before each update. This option should
+            only be used if the imported package failes to re-render when props change.
+            Using this option has negative performance consequences since all DOM
+            elements must be changed on each render. See :issue:`461` for more info.
     """
     source_file = Path(file)
     target_file = _web_module_path(name)
@@ -179,6 +199,7 @@ def module_from_file(
             if resolve_exports
             else None
         ),
+        unmount_before_update=unmount_before_update,
     )
 
 
@@ -189,6 +210,7 @@ class WebModule:
     default_fallback: Optional[Any]
     export_names: Optional[Set[str]]
     file: Optional[Path]
+    unmount_before_update: bool
 
 
 @overload
@@ -250,7 +272,10 @@ def export(
 
 
 def _make_export(
-    web_module: WebModule, name: str, fallback: Optional[Any], allow_children: bool
+    web_module: WebModule,
+    name: str,
+    fallback: Optional[Any],
+    allow_children: bool,
 ) -> VdomDictConstructor:
     return partial(
         make_vdom_constructor(
@@ -261,6 +286,7 @@ def _make_export(
             source=web_module.source,
             sourceType=web_module.source_type,
             fallback=(fallback or web_module.default_fallback),
+            unmountBeforeUpdate=web_module.unmount_before_update,
         ),
     )
 

src/idom/web/utils.py

@@ -11,6 +11,8 @@
 
 
 def module_name_suffix(name: str) -> str:
+    if name.startswith("@"):
+        name = name[1:]
     head, _, tail = name.partition("@")  # handle version identifier
     version, _, tail = tail.partition("/")  # get section after version
     return PurePosixPath(tail or head).suffix or ".js"

tests/test_server/test_common/test_per_client_state.py

@@ -66,7 +66,7 @@ def Counter():
 
 
 def test_module_from_template(driver, display):
-    victory = idom.web.module_from_template("react", "[email protected]")
+    victory = idom.web.module_from_template("react", "victory-bar@35.4.0")
     VictoryBar = idom.web.export(victory, "VictoryBar")
     display(VictoryBar)
     driver.find_element_by_class_name("VictoryContainer")

tests/test_web/test_module.py

@@ -79,7 +79,7 @@ def test_module_from_template_where_template_does_not_exist():
 
 
 def test_module_from_template(driver, display):
-    victory = idom.web.module_from_template("react", "[email protected]")
+    victory = idom.web.module_from_template("react", "victory-bar@35.4.0")
     VictoryBar = idom.web.export(victory, "VictoryBar")
     display(VictoryBar)
     wait = WebDriverWait(driver, 10)