Improve 'relink private keys' steps

[etpinard]

Synopsis

At the moment, during Plots.supplyDefaults we loop over all keys found in the old trace and layout containers in order to relink all private keys not set during the defaults step in the new trace and layout containers (remember, fullData and fullLayout start as empty [] and {} respectively in every Plots.supplyDefaults call).

These private keys are linked to subplot instances (e.g fullLayout.scene._scene), computed values (e.g. fullLayout.xaxis._m), graph svg selections (e.g. fullLayout._infolayer) etc ...

In brief

Looping over all keys is dumb, slow and hard to maintain ; we can do better!

Observations

Not relinking private keys in trace objects results in:

• geo trace visibility toggle not functioning (_module isn't defined anymore for visible: false traces)
• contour restyle not working as trace._emptypoints isn't relinked

Not relinking array container results in:

• annotations not behaving correctly on zoom -> double-click interactions

Possible solution

Make the trace and base plot module (and possibly the component module) clean method relink the sticky keys that need to be relinked during Plots.supplyDefaults. This would make the clean methods handle all old container --> new container logic in one go.

For example, most base plot modules have a clean method that looks like:

oldSubplotIds.forEach((id) => {
  if(!newFullLayout[id]._subplot && !!oldFullLayout[id]._subplot) {
     oldFullLayout[id]._subplot.destroy();
     return;
  }

  // it would be then be easy to relink the active subplot objects here
  newFullLayout[id]._subplot = oldFullLayout[id]._subplot;
});

It might be convenient to put private keys generated in makePlotFramework under one object in fullLayout (e.g fullLayout_base._toppaper) to keep track of private keys more easily.

[etpinard]

Closing this thing for the time being. relinkPrivateKeys is getting better and more thoroughly tested, thanks to notably #2375.

[alexcjohnson]

Reopening - I've lost track of the pattern (if there is one) for which keys need relinking, but as #2375 and #2465 show there are real downsides to the "relink everything" approach. I really like the approach of having clean handle this, but it still may take a bunch of extra testing to do this safely.

[gvwilson]

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson