diff --git a/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx b/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx index 7172685c222..d116acec5af 100644 --- a/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx +++ b/flow-server/src/main/resources/com/vaadin/flow/server/frontend/Flow.tsx @@ -208,22 +208,10 @@ type PortalEntry = { type FlowPortalProps = React.PropsWithChildren< Readonly<{ domNode: HTMLElement; - onRemove(): void; }> >; -function FlowPortal({ children, domNode, onRemove }: FlowPortalProps) { - useEffect(() => { - domNode.addEventListener( - 'flow-portal-remove', - (event: Event) => { - event.preventDefault(); - onRemove(); - }, - { once: true } - ); - }, []); - +function FlowPortal({ children, domNode }: FlowPortalProps) { return createPortal(children, domNode); } @@ -378,15 +366,27 @@ function Flow() { (event: CustomEvent) => { event.preventDefault(); + const { domNode, children } = event.detail; const key = Math.random().toString(36).slice(2); + + // Register the removal listener synchronously, not from an effect + // inside FlowPortal: the portal renders asynchronously, so a + // 'flow-portal-remove' dispatched before the portal is committed + // (e.g. when a dialog moves the element right after attaching it) + // would be missed, leaving a duplicate portal and a double render. + domNode.addEventListener( + 'flow-portal-remove', + (removeEvent: Event) => { + removeEvent.preventDefault(); + dispatchPortalAction(removeFlowPortal(key)); + }, + { once: true } + ); + dispatchPortalAction( addFlowPortal( - dispatchPortalAction(removeFlowPortal(key))} - > - {event.detail.children} + + {children} ) ); diff --git a/flow-tests/test-react-adapter/src/main/java/com/vaadin/flow/ReactAdapterView.java b/flow-tests/test-react-adapter/src/main/java/com/vaadin/flow/ReactAdapterView.java index 0bc6673782b..c243c76eeb1 100644 --- a/flow-tests/test-react-adapter/src/main/java/com/vaadin/flow/ReactAdapterView.java +++ b/flow-tests/test-react-adapter/src/main/java/com/vaadin/flow/ReactAdapterView.java @@ -46,8 +46,35 @@ public ReactAdapterView() { (event) -> getOutput.setText(input.getValue())); getValueButton.setId("getValueButton"); + // Reproduces the dialog scenario where the adapter element is attached + // and then immediately moved in the DOM (disconnect + reconnect) within + // the same task, before React has committed the portal. A single React + // component must be rendered, not one per connect. + var moveTarget = new Div(); + moveTarget.setId("moveTarget"); + var moveButton = new NativeButton("Move while connecting"); + moveButton.setId("moveWhileConnectingButton"); + moveButton.getElement().addEventListener("click", + event -> moveTarget.getElement().executeJs( + // language=JavaScript + """ + const target = $0; + target.textContent = ''; + const first = document.createElement('div'); + const second = document.createElement('div'); + target.append(first, second); + const adapter = document.createElement('react-input'); + // First connect schedules a portal asynchronously. + first.appendChild(adapter); + // Moving it synchronously disconnects and reconnects it + // before the portal is committed. + second.appendChild(adapter); + """, + moveTarget.getElement())); + add(new Div(input, listenerOutput), new Div(setValueButton), - new Div(setNullButton), new Div(getValueButton, getOutput)); + new Div(setNullButton), new Div(getValueButton, getOutput), + new Div(moveButton), moveTarget); } } diff --git a/flow-tests/test-react-adapter/src/test/java/com/vaadin/flow/ReactAdapterIT.java b/flow-tests/test-react-adapter/src/test/java/com/vaadin/flow/ReactAdapterIT.java index 2df64cda400..26f1463a629 100644 --- a/flow-tests/test-react-adapter/src/test/java/com/vaadin/flow/ReactAdapterIT.java +++ b/flow-tests/test-react-adapter/src/test/java/com/vaadin/flow/ReactAdapterIT.java @@ -94,6 +94,24 @@ public void validateSetNullState() { Assert.assertNull("Expected null value, not string 'null'", value); } + @Test + public void moveWhileConnecting_rendersComponentOnce() { + open(); + + waitForDevServer(); + + $(NativeButtonElement.class).id("moveWhileConnectingButton").click(); + + TestBenchElement moveTarget = $(TestBenchElement.class) + .id("moveTarget"); + waitUntil(driver -> !moveTarget.$("react-input").all().isEmpty()); + + Assert.assertEquals( + "Adapter element moved while connecting must render exactly one " + + "React component", + 1, moveTarget.$("input").all().size()); + } + private TestBenchElement getAdapterElement() { return $("react-input").first(); }