holochain / holochain-client-js

A JavaScript client for the Holochain Conductor API
Other
258 stars 15 forks source link

AppSignal missing metadata (cell and zome info) #98

Closed nphias closed 1 year ago

nphias commented 2 years ago

the AppSignal object returned by the signal_handler callack does not tell us which cell (nick/role) or which zome the signal came from .. I see this is as a bug now, rather than a feature request, so makes sense to create an issue here

I believe others are beginning to realise too now that creating a front-end architecture to mirror what is happening in the back-end such that signals from multi-cell , multi-zome happs can be routed effecively to front end components in a way that does not constrain zome developers and facilities re-use of zomes is an obvious way forward..

nphias commented 1 year ago

from:

export type AppSignal = {
  type: string;
  data: {
    cellId: CellId;
    payload: any;
  };
};

To:

export type AppSignal = {
  name: string;  // user defined in zome (signal.name)
  type: string; // ENUM defined in zome (signal.type) 
  data: {
    cellId: CellId;     //metadata
    zomeid: ZomeId    //metadata
    payload: any;
  };
};
jost-s commented 1 year ago

Hi Thomas, I'll line this addition up for implementation. This requires a change in Holochain too. FYI @abe-njama

maackle commented 1 year ago

I'm confused by what's proposed here, since the AppSignal implementation is just an opaque type: https://github.com/holochain/holochain/blob/7eb4275e7a5a64cb671995cd30149abaeca55dba/crates/holochain_zome_types/src/signal.rs#L10

I agree that adding extra context into the signal could be useful/convenient, but it also seems that for now you could work around this by including whatever info you need into the signal itself, right? Including making an enum with various types of signals? Therefore I'm not sure this is a bug?

guillemcordoba commented 1 year ago

@maackle the actual change that we could do here is adding the zome name to the signal data, here: https://github.com/holochain/holochain/blob/97c2a262ac76725d21de99314962e21a210ec6db/crates/holochain_types/src/signal.rs#L13. And extra points for adding the rolename alongside the CellId. That should be easy no?

nphias commented 1 year ago

I'm confused by what's proposed here, since the AppSignal implementation is just an opaque type: https://github.com/holochain/holochain/blob/7eb4275e7a5a64cb671995cd30149abaeca55dba/crates/holochain_zome_types/src/signal.rs#L10

I agree that adding extra context into the signal could be useful/convenient, but it also seems that for now you could work around this by including whatever info you need into the signal itself, right? Including making an enum with various types of signals? Therefore I'm not sure this is a bug?

imagine we are trying to re-use zomes in a zome library.. but every developer has invented their own signal ontology.. with no standard how can anyone write any meaningful front-end code to route signals without having to fork every zome they use to fit their unified signal structure?

nphias commented 1 year ago

what if i want a signal that tells me someone has created a new cell clone?

jost-s commented 1 year ago

The zome name has been added in #166. https://github.com/holochain/holochain/blob/b60c64ec375f88670a53af11edd160047f37eff7/crates/holochain_conductor_api/src/app_interface.rs#L257-L272

If there's anything else you need, please re-open or create a new issue, @nphias.