with-d3-require - refresh + error handling

[mhuebert]

~with-d3-require already serves as a kind of error-boundary. With this change, the f we pass in receives an extra param wrap-callback which can be used to wrap event handlers so that any errors occurring within (async or sync) are handled in the error boundary.~

~Also using :key as a prop instead of metadata (to ensure the component re-renders when the value changes), maybe feels better?~

Addresses the issues of (1) d3-require staying mounted & not losing its height when re-rendering/refreshing, (2) how to handle async errors in a :ref function.

• Use reagent functional components for sci-viewers
• error-boundary provides :!error as react context
• use-handle-error hook so that child views can write to the nearest error handler manually
• add use-d3-require hook
• simplify with-d3-require: no need for a :key - the component stays mounted and re-fetches packages whenever the args change. the invalidation now occurs where we pass deps to useEffect

[mhuebert]

After a chat with @mk tried another route - using hooks and a function component. It's not particularly ergonomic to do with reagent but much easier to achieve exactly what we want. Fixed the issue with the height of the view changing in-between renders. Now it accepts :refresh-key to determine when to re-render.

But the original issue - async error catching - is not handled in this version, we now pass our !error atom down via react context, so we should access that and use it with the ref callback. Didn't have time yet.

[mhuebert]

I added the async error handler back in, using a hook, and to simplify things - I changed the default reagent compiler to functional components. So this would now need more testing. But supporting hooks IMO is very useful.

[mhuebert]

further re-implemented everything using hooks, much simpler and composed of small parts.

[mk]

Very nice. There still seems to be an issue with Plotly in the static build still, see https://snapshots.nextjournal.com/clerk/build/88e94fce8b12fd4c6770ecfea60f616fbb00388f/index.html#/notebooks/viewers/plotly.clj

Locally I saw an Rendered more hook than on the previous render error, but I can't reproduce it reliably not sure if that's related or just caused by me pulling from a state without hooks.

[mhuebert]

@mk I also see that hook error locally when switching between different notebooks. Looking into it.

[mhuebert]

I found two issues -

• an advanced-compile issue that was fixed by adding a return-type hint to use-d3-require,
• an issue with how inspect-presented was calling viewer functions, which wasn't giving each "viewer" its own React component instance (& therefore, its own hook state). Fixed by adding a 3rd arity to inspect-presented which is invoked as a component with ^{:key viewer} so that viewers get hook state which is persisted across re-render. https://github.com/nextjournal/clerk/pull/231/commits/b883522867064679c22381e6c0bf469a9eb4985c#diff-2eb22f30e4f63ae4ccc753c326e323e3cd1b0bd273ca1655399f12b033da2a64R570

[mk]

@mhuebert just getting around now to play with this, it's SO much better than what we had before, both in experience (without the height flickering) and code. Would love to go over the inspect-presented change on a call but that can happen independently of the merge.