Fix proxy this for emittery

[ThetaSinner]

This addresses #195

[ThetaSinner]

I've struggled to reproduce this in Node tests. I can do it with JavaScript but whatever is happening there, TypeScript seems to fix. Without digging deeper into the Vue.js source I'm not sure what they are doing differently. Maybe somebody with better TypeScript knowledge can figure this one out.

I have installed this PR into my own project and it does fix the problem.

[ThetaSinner]

Not sure what's happening with the macos build, is this a new problem that's worth me digging into or something known that we can address?

[jost-s]

Not sure what's happening with the macos build, is this a new problem that's worth me digging into or something known that we can address?

That's a problem with the nix install action. I upgraded it, that should fix it.

[ThetaSinner]

Awesome, thank you!

[jost-s]

@ThetaSinner Why do you want an AppAgentWs to be reactive? I see that @mattyg did that in mewsfeed, but I cannot see why that's necessary there, either. It seems like the AppAgentWs can just be a variable with let and doesn't need to be reactive.

[mattyg]

@ThetaSinner Why do you want an AppAgentWs to be reactive? I see that @mattyg did that in mewsfeed, but I cannot see why that's necessary there, either. It seems like the AppAgentWs can just be a variable with let and doesn't need to be reactive.

It needs to be reactive so I can initialize it in the vue app's root component. Then I can display content while waiting for it to initialize, and when its ready, reactively make it available to sub-components via provide / inject. So it needs to reactively update the client variable from undefined to the actual AppAgentWs. (also this is how the scaffolder implements it)

I just ran into this issue in another context where I am passing the AppAgentWebsocket into new class as a constructor param, and then trying to listen to signals within that class. Unfortunately this PR doesn't seem to resolve it in that case.

[ThetaSinner]

@ThetaSinner Why do you want an AppAgentWs to be reactive? I see that @mattyg did that in mewsfeed, but I cannot see why that's necessary there, either. It seems like the AppAgentWs can just be a variable with let and doesn't need to be reactive.

@jost-s Yes to what @mattyg said and I'm going to expand a little bit.

• I don't want the app to have to wait to render until the client is connected, all my structural components can render then I can render placeholders for anything that needs data until the client is connected.
• I don't want to have to reload the page if the connection to the back-end drops for any reason. I think it's simpler to reconnect a websocket and resume sending requests than it is to persist and recreate client state on reload.
• I don't want to pass the client through all my components in either React or Vue (I don't know anything about Svelte). In Vue provide/inject is the mechanism for deep passing, in React that would be a hook that provides the client. In either case the provide should be reactive so that the UI can proceed with rendering without waiting for a value.

I'm still experimenting but I've actually already taken this a step further and wrapped the client. This lets me transparently reconnect the client which I can't do with this client because construction/connection is atomic. So I am injecting my client wrapper into the components that need it then they go ahead and render while creating a promise on waitForConnected that let's them fill or create in data once a client is available, regardless of which connection that is under the hood.

[ThetaSinner]

I just ran into this issue in another context where I am passing the AppAgentWebsocket into new class as a constructor param, and then trying to listen to signals within that class. Unfortunately this PR doesn't seem to resolve it in that case.

@mattyg any chance you can point me at an example and I'll see if I can figure out why this isn't working everywhere?

[mattyg]

I just ran into this issue in another context where I am passing the AppAgentWebsocket into new class as a constructor param, and then trying to listen to signals within that class. Unfortunately this PR doesn't seem to resolve it in that case.

@mattyg any chance you can point me at an example and I'll see if I can figure out why this isn't working everywhere?

Well I tried to make an example, but it worked properly, and then I fixed it in my use case as well, so nevermind!

[ThetaSinner]

@mattyg Cool okay, if it pops up again feel free to send it my way :)

[jost-s]

@ThetaSinner With the wrapper, do you still need the Emittery workaround?

[jost-s]

@ThetaSinner I suppose so. I'll merge this, thanks you for the work!

[ThetaSinner]

Sorry I hadn't got to this yet but yes, the Vue proxies are deep so even with a wrapper the this will move.

[jost-s]

Sorry I hadn't got to this yet but yes, the Vue proxies are deep so even with a wrapper the this will move.

Right, okay. I have been unable to reproduce the problem too, even in a JS file using Vue's ref function.

[ThetaSinner]

The code I used to test the fix is

import Emittery from 'emittery';

class T extends Emittery {
  constructor() {
    super();

    // // Ensure all super methods are bound to this instance because Emittery relies on `this` being the instance.
    // Object.getOwnPropertyNames(Emittery.prototype).forEach((name) => {
    //     if (typeof this[name] === 'function') {
    //         this[name] = this[name].bind(this);
    //     }
    // });
  }

  doOn(func) {
    console.log(this);
    this.on('hello', func);
  }
}

let t = new Proxy(new T(), {});
t.doOn(() => {
    console.log('got a hello');
});

t.emit('hello');

It can just be run with node and the fix can be commented in.

I get the same Emittery error I got from a Vue app

file:///home/thetasinner/source/holo/holochain-client-js/node_modules/emittery/index.js:27
        if (!events.has(eventName)) {

[jost-s]

Thanks for that. Do you have a clue why something like

const appAgentWs = await AppAgentWs.connect(...);
const ws = ref(appAgentWs);
ws.value.client.on('signal', () => console.log('signalled'));
ws.value.client.emit('signal', ...);

works?

[ThetaSinner]

That does fail for me, if I do npm i -D vue and create a Javascript file called test2.js containing

import { AppAgentWebsocket } from './lib/index.js';
import { ref } from 'vue';

const appAgentWs = await AppAgentWebsocket.connect('ws://localhost:8080');
const ws = ref(appAgentWs);
ws.value.on('signal', (m) => console.log('signalled', m));
ws.value.emitter.emit('signal', {
    test: 'message'
});

I can then start a sandbox, run it with hc sandbox run -p 8080 and run that script against it, I get an error

file:///home/thetasinner/source/holo/holochain-client-js/node_modules/emittery/index.js:27
        if (!events.has(eventName)) {
                    ^

TypeError: Cannot read properties of undefined (reading 'has')
    at getListeners (file:///home/thetasinner/source/holo/holochain-client-js/node_modules/emittery/index.js:27:14)
    at Proxy.on (file:///home/thetasinner/source/holo/holochain-client-js/node_modules/emittery/index.js:280:14)
    at Proxy.on (file:///home/thetasinner/source/holo/holochain-client-js/lib/api/app-agent/websocket.js:171:29)
    at file:///home/thetasinner/source/holo/holochain-client-js/test2.js:6:10
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

The test script runs fine if I comment my fix back in