Refresh plan statistics after re-run the query

[alexabidri]

Problem has been explained here -> #1454

The issue is to refresh the state of the react component that display the plan stats at the bottom of the window

Here is a little video of the fix, sorry for the bad quality, you can see the stats are updating at the bottom of the query window

[OskarDamkjaer]

Hi! Thanks for both reporting and fixing this issue! We're already aware of it and I put a fix up in #1451, but I'm happy to include your fix instead.

The bug snuck in as we didn't want to run bolt.extractPlan unless it was needed, as you noted this was done in a way so that only ever ran once. With your suggested fix, it'll be recalculated every time the component re-renders, which would be less performant but not an issue for such a simple component. However it seems to me that we no longer need the componentDidMount and the componentDidUpdate lifecycle methods when we have derivedStateFromprops?

Also for the tests to run properly and for us to be able to merge your change you'll need to sign our CLA.

[OskarDamkjaer]

To prevent the performance hit, we could add memoize-one with a custom equality function using the check from shouldComponentUpdate. What do you think @alexabidri?

[alexabidri]

@OskarDamkjaer Thanks for your comment and happy to consider my fix!
Will take into account and re-test the flow 👍
Also, I have sent the proper CLA but it still doesnt pass the CI

[OskarDamkjaer]

There's a manual step to whitelist contributors, didn't see your email but I've added you now!

I had a look at your code, and the memorization is not quite there yet, it'll still run extractPlan when it's not needed. For it to only rerun when we the summary changes we'll need to do something like this (custom equality function):

const extractMemoizedPlan = memoize(
  result => bolt.extractPlan(result, true),
  (a: any, b: any) => deepEquals(a[0]?.result?.summary, b[0]?.result?.summary)
)

Reading your changes I found we also no longer need to keep the plan in state, so we could just do something like this in the render():

const { result } = this.props
if (!result || !result.summary) return null

const plan = extractMemoizedPlan(result)
if (!plan) return null

return (<StyledOneRowStatsBar>//... and so on

Are you happy to update the memo function?

[alexabidri]

You are right, will be even better indeed. I took your feedback into account and also passed the component as a pure component for readability and understandability. Thanks

[OskarDamkjaer]

Nice cleanup, thanks a lot! I'll merge when the tests come back green 🎉

[OskarDamkjaer]

Thanks for this! 💯

[OskarDamkjaer]

If you want to use your changes before they're officially released in browser 4.3.2 (some time in august), master is deployed to https://browser-canary.graphapp.io/ a few hours after merging. So you should be able to use it from there tomorrow (not guaranteed to be 100% stable though 😅 )