JoshMarler / react-juce

Write cross-platform native apps with React.js and JUCE
https://docs.react-juce.dev
MIT License
763 stars 79 forks source link

console.log(syntheticEvent) causes "caller attempted to invoke a non-existent view method" error #210

Open androidStern opened 3 years ago

androidStern commented 3 years ago

repro steps:

  1. add an onMouseDrag callback to your react component that simply console.logs the drag event. In my case my component was a wrapper around a button. For example:
    onMouseDrag = console.log
  2. run your app and initiate a drag event

    You'll see something like the following error:

image

Additional Context

I added a breakpoint right before the exception and noticed it was a call to toJSON that was the "non-existent view method"

image

And patching SyntheticMouseEvent.toJSON fixes the issue. Something like:

SyntheticMouseEvent.prototype.toJSON = ()=>{
  let o = {
    x: this.x,
    y: this.y,
    screenX: this.screenX,
    screenY: this.screenY,
  }

  return JSON.stringify(o)
}

For additional context see this Discord thread

ilionic commented 3 years ago

Based on React SyntheticEvent doc that's not valid event:

onClick onContextMenu onDoubleClick onDrag onDragEnd onDragEnter onDragExit
onDragLeave onDragOver onDragStart onDrop onMouseDown onMouseEnter onMouseLeave
onMouseMove onMouseOut onMouseOver onMouseUp

https://reactjs.org/docs/events.html#mouse-events

So react-juce provides own implementation of it?

nick-thompson commented 3 years ago

@ilionic that's right, we currently have a non-standard onMouseDrag feature: https://github.com/nick-thompson/blueprint/blob/master/react_juce/core/View.cpp#L249. I think in the long run we will want to remove it for closer parity to the web standards, but for now it's in there.

The problem is not that we're missing a toJSON (this is a non-standard Duktape feature in itself), but that our synthetic event proxy gets confused somewhere along the event dispatching code path. This is a bit of an involved issue, but it will provide a very good learning experience if you want to get into it :)

If you follow along from that line in View.cpp that I linked above, you'll find that we walk through a few cpp helper calls before eventually landing right here in our javascript lib: https://github.com/nick-thompson/blueprint/blob/master/packages/react-juce/src/lib/Backend.ts#L222. This is how we make sure to invoke whatever onMouseDrag prop you've provided on your view component. Now the interesting thing is, these SyntheticEvents that we create and dispatch here have no way of accessing this "invoke view method" feature that @androidStern highlighted above. In fact, the goal of that API is more here: https://github.com/nick-thompson/blueprint/blob/master/packages/react-juce/src/lib/Backend.ts#L123, so that you can call into native view methods on the object that we give back to you when you use a viewRef.

So right there we have the problem: the synthetic view events that we bubble up to the user callback props don't get wrapped in this proxy that I highlighted on line 123, so how are we landing at a call to invokeViewMethod? Something in there is going wrong

Let me know if that provides helpful context, I can elaborate further if it's helpful, but worth knowing that this issue will certainly involve going pretty deep into the internals here!

ilionic commented 3 years ago

Will post some findings. Don't have a full picture yet and continue digging but brains already fried :)

Found that such behaviour of onMouseDrag happens only on components with ref, ie Button. Slider on contrary works fine. NativeMethods being called because prop toJSON is not in the target. Target is React ref containing following properties: ["_props","_parent","_id","_type","_children","contains"] Why toJSON appears there at all I think lies somewhere deep in Duktape console proxying and I don't really have an answer for it.

nick-thompson commented 3 years ago

Great @ilionic sounds like you're on the right path definitely. toJSON is indeed part of Duktape's console proxy stuff; I don't think we need to worry about that part too much– the real question there is "why are we eventually trying to call toJSON on our native method proxy?" Seems like we're giving the wrong thing back to a ref or something like that...