[Canvas] Only fetch saved elements once

[crob611]

Summary

Saw this bug where there was no requirements on a useEffect so when you went to add a Custom Element, it would repeatedly request the custom elements.

This adds requirements to the useEffect and adds a ref that gets flipped because the findCustomElements function is a new function on every rerender 😦

[elasticmachine]

Pinging @elastic/kibana-canvas (Team:Canvas)

[clintandrewhall]

Nice catch, but the bug is that useEffect should have an empty array as a dependency list, rather than no list at all:

If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument. This tells React that your effect doesn’t depend on any values from props or state, so it never needs to re-run. This isn’t handled as a special case — it follows directly from how the dependencies array always works.

As such, you don't need the ref.

https://reactjs.org/docs/hooks-effect.html

EDIT: I'm betting this is going to trigger the exhausting-deps rule, though... which is probably why this is missing that array in the first place. :-/

[crob611]

@clintandrewhall Yep, the exhaustive-deps led me to the ref way of accomplishing this.

Should we go forward with this way, or intentionally disable exhaustive-deps and use the empty array to get the functionality we actually want?

[clintandrewhall]

Let's go with this for now... we need the bug fix. Let's look into some better ways to do this/refactor the props to not need a function/etc for the next release.

[crob611]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 7a206e0

Build metrics

✅ unchanged

History

• 💔 Build #60431 failed 3f80a8d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream