fix(toolbar): Guard against adding toolbar to DOM twice

[msfstef]

Checks whether container element has already been added before proceeding to add toolbar.

With React's StrictMode and other cases the addToolbar call might occur twice, handling it with a warning I believe is better than letting it add a broken DOM element.

[samwillis]

👍

[thruflo]

Is this dom checking approach really the best solution? Just strikes me that presumably this is an indication that we’re doing something non idiomatic.

Why is rendering the toolbar twice problematic in the first place?

[samwillis]

@thruflo

I think this is a good way to guard in this situation.

Say a user embeds two separate Electric apps in a page, and both have their own copy of the toolbar in dev mode. It's possible that there are two separate bundled versions of the toolbar loaded. Using the DOM as a guard prevents the toolbar being added twice from the two copies.

The alternative is to save a global on the window object, but I think I prefer this method.

The double rendering (so far) is a side effect of react strict mode. The toolbar isn't part of the users react app, it's a separate react app that is injected into the body (and so works with Vue and Svelte or anything else). If the user initiates it in a useEffect then under strict mode it injected twice.

Essentially, the guard needs to be global, and outside of the scope of the toolbar package. So either the dom or window object.

[thruflo]

I get it, I just don't get why the toolbar can't handle unmount, clean itself up and be mounted again.

[msfstef]

@thruflo that's also a possibility if we think that gives a better DX - I'd still keep a warning though since I would imagine in most cases it would be accidental (like using React.StrictMode) and probably annoying since it will minimize the toolbar as well if you're looking at it.

Either way here's a PR modifying this fix to allow for proper remounting #1287