Don't retain state across distinct-but-similar expanders (#4290)

* Don't retain state across distinct-but-similar expanders

* Add explanatory comment

e2e/scripts/expander_state.py

@@ -0,0 +1,12 @@
+import streamlit as st
+
+b0 = st.button("b0")
+b1 = st.button("b1")
+
+if b0:
+    with st.expander("b0_expander", expanded=False):
+        st.write("b0_write")
+
+if b1:
+    with st.expander("b1_expander", expanded=False):
+        st.write("b1_write")

e2e/specs/expander_state.spec.js

@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright 2018-2022 Streamlit Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+describe("expandable state", () => {
+  before(() => {
+    cy.loadApp("http://localhost:3000/");
+  });
+
+  it("does not retain expander state for a distinct expander", () => {
+    cy.getIndexed(".stButton button", 0).click();
+    cy.get("[data-testid='stExpander']").click();
+
+    cy.get("[data-testid='stExpander']").should("contain.text", "b0_write");
+
+    cy.getIndexed(".stButton button", 1).click();
+
+    cy.get("[data-testid='stExpander']").should(
+      "not.contain.text",
+      "b0_write"
+    );
+    cy.get("[data-testid='stExpander']").should(
+      "not.contain.text",
+      "b1_write"
+    );
+  });
+});

frontend/src/hocs/withExpandable/withExpandable.tsx

@@ -48,12 +48,20 @@ function withExpandable(
       ...componentProps
     } = props
 
-    const [expanded, toggleExpanded] = useState<boolean>(initialExpanded)
+    const [expanded, setExpanded] = useState<boolean>(initialExpanded)
     useEffect(() => {
-      toggleExpanded(initialExpanded)
-    }, [initialExpanded])
+      setExpanded(initialExpanded)
+      // Having `label` in the dependency array here is necessary because
+      // sometimes two distinct expanders look so similar that even the react
+      // diffing algorithm decides that they're the same element with updated
+      // props (this happens when something in the app removes one expander and
+      // replaces it with another in the same position).
+      //
+      // By adding `label` as a dependency, we ensure that we reset the
+      // expander's `expanded` state in this edge case.
+    }, [label, initialExpanded])
 
-    const toggle = (): void => toggleExpanded(!expanded)
+    const toggle = (): void => setExpanded(!expanded)
     const { colors, radii, spacing, fontSizes } = useTheme<Theme>()
 
     return (