Tooltip closing race condition with children change callback

[FRosner]

Tooltip closing race condition with children change callback

[FRosner]

I wonder if this is caused by fade or delay? I'm trying to find the code that updates the is_open erroneously... @tcbegley can you help me?

Is it here?

dash-bootstrap-components/src/private/Overlay.js

Lines 62 to 80 in 71b3070

	const hide = () => {
	if (isOpenRef.current) {
	hideTimeout.current = clearTimeout(hideTimeout.current);
	setIsOpen(false);
	if (setProps) {
	setProps({is_open: false});
	}
	}
	};
	// Function to create the delay for hiding if currently open
	const hideWithDelay = () => {
	if (!isOpenRef.current && showTimeout.current) {
	showTimeout.current = clearTimeout(showTimeout.current);
	hide();
	} else if (isOpenRef.current) {
	clearTimeout(hideTimeout.current);
	hideTimeout.current = setTimeout(hide, delay.hide);
	}
	};

If it's delay, setting delay={"show": 0, "hide": 0}, should be a reasonable workaround.

[tcbegley]

Hi @FRosner,

Thanks for reporting this and for providing so much detail and a minimal reproduction.

The issue is indeed in hideWithDelay. When the "close" even happens, we call setProps({is_open: false}) with a delay. In this case in the intervening time the component is unmounted. That means when Dash tries to update the prop the component no longer exists.

I think the fix here will be to either:

1. modify the timeout so that the call to setProps only happens if the component hasn't been unmounted
2. cancel outstanding timeouts when the component unmounts

I think 2) is probably cleaner but looking over the code briefly might require a bit of refactoring. I'll see what I can do.

[FRosner]

I'll see what I can do.

Thank you so much!

[FRosner]

I think the fix here will be to either:

To me, 1 sounds preferrable, as it has limited side effects and makes it clearer when the function is passed to setTimeout that this is something to be aware of. Why do you think 2 is cleaner?

[tcbegley]

Why do you think 2 is cleaner?

Well the timeouts only make sense in the context of the component, so if the component no longer exists it seems neater to stop them from running at all rather than to run and do little / nothing. But tbh both options are similar so I'll take whatever solution is simpler to implement.

I did have a go at getting this to work yesterday but it turned out to be fiddlier than expected. I might need to refresh some of my react knowledge and try again when I get a chance.

[FRosner]

Thanks! I am trying the workaround of setting hide delay to 0 in the meantime. I'd also take a stab at it but last time I used react is 5 years ago :D