[REF-3016] Allow special characters in upload ID (#3449)

reflex-dev · Jun 12, 2024 · 8c8156f · 8c8156f
1 parent c3c06a1
commit 8c8156f
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 10 deletions.
diff --git a/reflex/components/core/upload.py b/reflex/components/core/upload.py
@@ -50,10 +50,18 @@ def upload_file(id_: str = DEFAULT_UPLOAD_ID) -> BaseVar:
     Returns:
         A var referencing the file upload drop trigger.
     """
+    id_var = Var.create_safe(id_, _var_is_string=True)
+    var_name = f"""e => setFilesById(filesById => {{
+    const updatedFilesById = Object.assign({{}}, filesById);
+    updatedFilesById[{id_var._var_name_unwrapped}] = e;
+    return updatedFilesById;
+  }})
+    """
+
     return BaseVar(
-        _var_name=f"e => setFilesById(filesById => ({{...filesById, {id_}: e}}))",
+        _var_name=var_name,
         _var_type=EventChain,
-        _var_data=upload_files_context_var_data,
+        _var_data=VarData.merge(upload_files_context_var_data, id_var._var_data),
     )
 
 
@@ -67,10 +75,11 @@ def selected_files(id_: str = DEFAULT_UPLOAD_ID) -> BaseVar:
     Returns:
         A var referencing the list of selected file paths.
     """
+    id_var = Var.create_safe(id_, _var_is_string=True)
     return BaseVar(
-        _var_name=f"(filesById.{id_} ? filesById.{id_}.map((f) => (f.path || f.name)) : [])",
+        _var_name=f"(filesById[{id_var._var_name_unwrapped}] ? filesById[{id_var._var_name_unwrapped}].map((f) => (f.path || f.name)) : [])",
         _var_type=List[str],
-        _var_data=upload_files_context_var_data,
+        _var_data=VarData.merge(upload_files_context_var_data, id_var._var_data),
     )
 
 
@@ -99,7 +108,9 @@ def cancel_upload(upload_id: str) -> EventSpec:
     Returns:
         An event spec that cancels the upload when triggered.
     """
-    return call_script(f"upload_controllers[{upload_id!r}]?.abort()")
+    return call_script(
+        f"upload_controllers[{Var.create_safe(upload_id, _var_is_string=True)._var_name_unwrapped!r}]?.abort()"
+    )
 
 
 def get_upload_dir() -> Path:

diff --git a/reflex/event.py b/reflex/event.py
@@ -386,12 +386,12 @@ def as_event_spec(self, handler: EventHandler) -> EventSpec:
         )
 
         upload_id = self.upload_id or DEFAULT_UPLOAD_ID
-
         spec_args = [
             (
                 Var.create_safe("files", _var_is_string=False),
                 Var.create_safe(
-                    f"filesById.{upload_id}", _var_is_string=False
+                    f"filesById[{Var.create_safe(upload_id, _var_is_string=True)._var_name_unwrapped}]",
+                    _var_is_string=False,
                 )._replace(_var_data=upload_files_context_var_data),
             ),
             (

diff --git a/tests/components/forms/test_uploads.py b/tests/components/forms/test_uploads.py
@@ -39,6 +39,19 @@ def upload_component():
     return upload_component()
 
 
+@pytest.fixture
+def upload_component_id_special():
+    def upload_component():
+        return rx.upload(
+            rx.button("select file"),
+            rx.text("Drag and drop files here or click to select files"),
+            border="1px dotted black",
+            id="#spec!`al-_98ID",
+        )
+
+    return upload_component()
+
+
 @pytest.fixture
 def upload_component_with_props():
     """A test upload component with props function.
@@ -72,7 +85,12 @@ def test_upload_root_component_render(upload_root_component):
     assert upload["props"] == [
         "id={`default`}",
         "multiple={true}",
-        "onDrop={e => setFilesById(filesById => ({...filesById, default: e}))}",
+        "onDrop={e => setFilesById(filesById => {\n"
+        "    const updatedFilesById = Object.assign({}, filesById);\n"
+        "    updatedFilesById[`default`] = e;\n"
+        "    return updatedFilesById;\n"
+        "  })\n"
+        "    }",
         "ref={ref_default}",
     ]
     assert upload["args"] == ("getRootProps", "getInputProps")
@@ -114,7 +132,12 @@ def test_upload_component_render(upload_component):
     assert upload["props"] == [
         "id={`default`}",
         "multiple={true}",
-        "onDrop={e => setFilesById(filesById => ({...filesById, default: e}))}",
+        "onDrop={e => setFilesById(filesById => {\n"
+        "    const updatedFilesById = Object.assign({}, filesById);\n"
+        "    updatedFilesById[`default`] = e;\n"
+        "    return updatedFilesById;\n"
+        "  })\n"
+        "    }",
         "ref={ref_default}",
     ]
     assert upload["args"] == ("getRootProps", "getInputProps")
@@ -156,6 +179,27 @@ def test_upload_component_with_props_render(upload_component_with_props):
         "maxFiles={2}",
         "multiple={true}",
         "noDrag={true}",
-        "onDrop={e => setFilesById(filesById => ({...filesById, default: e}))}",
+        "onDrop={e => setFilesById(filesById => {\n"
+        "    const updatedFilesById = Object.assign({}, filesById);\n"
+        "    updatedFilesById[`default`] = e;\n"
+        "    return updatedFilesById;\n"
+        "  })\n"
+        "    }",
         "ref={ref_default}",
     ]
+
+
+def test_upload_component_id_with_special_chars(upload_component_id_special):
+    upload = upload_component_id_special.render()
+
+    assert upload["props"] == [
+        r"id={`#spec!\`al-_98ID`}",
+        "multiple={true}",
+        "onDrop={e => setFilesById(filesById => {\n"
+        "    const updatedFilesById = Object.assign({}, filesById);\n"
+        "    updatedFilesById[`#spec!\\`al-_98ID`] = e;\n"
+        "    return updatedFilesById;\n"
+        "  })\n"
+        "    }",
+        "ref={ref__spec_al__98ID}",
+    ]