Repository: marimo-team/marimo
Pull request: Fix: form internal value
This PR handles an edge case in which the form's wrapped element changes its value without firing an event, due to Python re-creating the form

Fixes #840 

Problem statement:
`ui.form()` initial value not updated when re-rendered
### Describe the bug

This issue was raised on Discord by @ztawil.

When a `form` element is re-rendered without unmounting/remounting, its initial value in the frontend isn't updated, causing it to return the incorrect value on submit.

This can be seen by populating the form's initial value with a UI element's value, changing the original UI element, and submitting the form.

### Environment

```
{
  "marimo": "0.2.8",
  "OS": "Linux",
  "OS Version": "6.5.0-15-generic",
  "Processor": "x86_64",
  "Python Version": "3.10.12",
  "Binaries": {
    "Chrome": "121.0.6167.184",
    "Node": "v18.13.0"
  },
  "Requirements": {
    "black": "23.3.0",
    "click": "8.1.3",
    "jedi": "0.19.1",
    "pymdown-extensions": "10.7",
    "starlette": "0.36.1",
    "tomlkit": "0.12.3",
    "typing_extensions": "4.9.0",
    "uvicorn": "0.27.0.post1"
  }
}
```

### Code to reproduce

```python
import marimo
 
__generated_with = "0.2.8"
app = marimo.App()
 
 
@app.cell
def __():
    import marimo as mo
    return mo,
 
 
@app.cell
def __(mo):
    mo.md("Modify the below text input to re-render the form, then hit submit")
    return
 
 
@app.cell
def __(mo):
    input = mo.ui.text()
    input
    return input,
 
 
@app.cell
def __(input, mo):
    input_form = mo.ui.text(value=input.value).form()
    input_form
    return input_form,
 
 
@app.cell
def __(input_form):
    input_form.value
    return
 
 
if __name__ == "__main__":
    app.run()
```

Hints from issue discussion:

Commit message: Fix: form internal value

Patch to review:
diff --git a/frontend/src/plugins/impl/FormPlugin.tsx b/frontend/src/plugins/impl/FormPlugin.tsx
index 1fa1d80e3a1..e93713b823c 100644
--- a/frontend/src/plugins/impl/FormPlugin.tsx
+++ b/frontend/src/plugins/impl/FormPlugin.tsx
@@ -63,7 +63,7 @@ export const FormPlugin = createPlugin("marimo-form")
       clearButtonLabel: z.string().default("Clear"),
       clearButtonTooltip: z.string().optional(),
       shouldValidate: z.boolean().optional(),
-    }),
+    })
   )
   .withFunctions<Functions>({
     validate: rpc
@@ -127,7 +127,7 @@ export const FormWrapper = <T,>({
           const response = await validate({ value: newValue }).catch(
             (error_) => {
               setError(error_.message ?? "Error validating");
-            },
+            }
           );
           if (response != null) {
             setError(response);
@@ -178,7 +178,7 @@ export const FormWrapper = <T,>({
               >
                 {clearButtonLabel}
               </Button>,
-              clearButtonTooltip,
+              clearButtonTooltip
             )}
           {withTooltip(
             <Button
@@ -189,7 +189,7 @@ export const FormWrapper = <T,>({
               {loading && <Loader2Icon className="h-4 w-4 mr-2 animate-spin" />}
               {submitButtonLabel}
             </Button>,
-            submitButtonTooltip,
+            submitButtonTooltip
           )}
         </div>
       </div>
@@ -216,7 +216,7 @@ const Form = ({
   // value of the plugin, which is the value of the wrapped plugin when
   // the submit button was last clicked/activated.
   const [internalValue, setInternalValue] = useState<T>(
-    UI_ELEMENT_REGISTRY.lookupValue(elementId),
+    UI_ELEMENT_REGISTRY.lookupValue(elementId)
   );
 
   // The Form may be rendered before the child plugin is, so after mount
@@ -225,6 +225,14 @@ const Form = ({
     setInternalValue(UI_ELEMENT_REGISTRY.lookupValue(elementId));
   }, [elementId]);
 
+  // Edge case: the Form plugin may be re-created in Python with the same
+  // wrapped `elementId`, meaning the value of the wrapped element
+  // can change without the plugin generating an event
+  const wrappedValue = UI_ELEMENT_REGISTRY.lookupValue(elementId);
+  if (UI_ELEMENT_REGISTRY.lookupValue(elementId) !== internalValue) {
+    setInternalValue(wrappedValue);
+  }
+
   useEffect(() => {
     // Spy on when the plugin generates an event (MarimoValueInputEvent)
     const handleUpdate = (e: MarimoValueInputEventType) => {


Code review comments:
diff_hunk: @@ -225,6 +225,14 @@ const Form = ({
     setInternalValue(UI_ELEMENT_REGISTRY.lookupValue(elementId));
   }, [elementId]);
 
+  // Edge case: the Form plugin may be re-created in Python with the same
+  // wrapped `elementId`, meaning the value of the wrapped element
+  // can change without the plugin generating an event
+  const wrappedValue = UI_ELEMENT_REGISTRY.lookupValue(elementId);
+  if (UI_ELEMENT_REGISTRY.lookupValue(elementId) !== internalValue) {
path: frontend/src/plugins/impl/FormPlugin.tsx
text: @user1:
could this be`if(wrappedValue !== internalValue)`

@author:
Oops yep. fixed