This is a fix for an bug with stack overflow in Athena and Redshift that happened when updating grafana to React 18, described here.
I wish I could say with any kind of authority why this happened in the first place, and only in variable editor. @sarahzinger already described here that the problem is in this useEffect:
if (!isEqual(props.dependencies, dependencies)) { setFetched(false); setResource(null); props.onChange(null); setDependencies(props.dependencies);
After debugging, it looks like the
props.query gets updated from props.onChange(null) BEFORE setDependencies(props.dependencies) is evaluated.
This triggers another run of the useEffect without the dependencies state variable having been updated to their latest value. This means the value is always undefined at the point when this effect runs.
That doesn't explain why it only happened in variable editor and not in panels though, but React's batching 🤷🏻♀️ amirite? 😞
I wanted to clean up the state and this effect a bit before I tackled the batching problem, so I followed what is usually recommended in the case of infinite useEffect loops, which is to reevaluate which variables are stored as state variables, and which are just refs. I moved basically all of them from this useEffect (fetched, resource, and dependencies) since they didn't need to be state variables and changes to them shouldn't cause a rerender (AFAI can see).
Aaand this ended up actually fixing the problem! It's the dependencies variable being changed to a ref that did it I think, I assume because it's no longer a state variable, it wasn't being updated (well, NOT updated) asynchronously?
Also, interestingly, if you replace this non-working useEffect with useLayoutEffect it works fine. Maybe it has to do with the synchronicity of useLayoutEffect, but my brain is fried and I welcome someone taking over the thinking for this, if you wish.
This might not be solving an overreaching problem, since I don't know the details of what caused it and in which cases. Im ok with hoping it doesn't happen again by being careful with what we put into state variables when there are a lot of moving parts (like here) from React@18 up.
This is a fix for an bug with stack overflow in Athena and Redshift that happened when updating grafana to React 18, described here.
I wish I could say with any kind of authority why this happened in the first place, and only in variable editor. @sarahzinger already described here that the problem is in this useEffect:
if (!isEqual(props.dependencies, dependencies)) { setFetched(false); setResource(null); props.onChange(null); setDependencies(props.dependencies);After debugging, it looks like the
dependenciesstate variable having been updated to their latest value. This means the value is always undefined at the point when this effect runs.That doesn't explain why it only happened in variable editor and not in panels though, but React's batching 🤷🏻♀️ amirite? 😞
I wanted to clean up the state and this effect a bit before I tackled the batching problem, so I followed what is usually recommended in the case of infinite useEffect loops, which is to reevaluate which variables are stored as state variables, and which are just refs. I moved basically all of them from this useEffect (
fetched, resource, and dependencies) since they didn't need to be state variables and changes to them shouldn't cause a rerender (AFAI can see).Aaand this ended up actually fixing the problem! It's the
dependenciesvariable being changed to a ref that did it I think, I assume because it's no longer a state variable, it wasn't being updated (well, NOT updated) asynchronously? Also, interestingly, if you replace this non-working useEffect with useLayoutEffect it works fine. Maybe it has to do with the synchronicity of useLayoutEffect, but my brain is fried and I welcome someone taking over the thinking for this, if you wish.This might not be solving an overreaching problem, since I don't know the details of what caused it and in which cases. Im ok with hoping it doesn't happen again by being careful with what we put into state variables when there are a lot of moving parts (like here) from React@18 up.
I also made a small change in CONTRIBUTING.md