Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify: What should happen on repeated listener addition for the same contextType/intent #1390

Open
1 of 6 tasks
kemerava opened this issue Oct 16, 2024 · 3 comments · May be fixed by #1394
Open
1 of 6 tasks

Clarify: What should happen on repeated listener addition for the same contextType/intent #1390

kemerava opened this issue Oct 16, 2024 · 3 comments · May be fixed by #1394
Labels
api FDC3 API Working Group question Further information is requested

Comments

@kemerava
Copy link
Contributor

Question Area

  • App Directory
  • API
  • Context Data
  • Intents
  • Use Cases
  • Other

Question

What should happen in the situation where for example app does:
await fdc3.addContextListener('fdc3.contact', (contact, metadata) => {... });
await fdc3.addContextListener('fdc3.contact', contact => { ... });
Or some different handler?

I see 3 possible options (in no particular order):

  1. Override the previous handler and keep the last one
  2. Keep and trigger both handlers (potential conflicts?)
  3. Throw an error and keep the original

Does the standard have any ruling on this, I was not able to find on the docs (please, let me know if I missed it)?

@kemerava kemerava added the question Further information is requested label Oct 16, 2024
@kriswest kriswest added the api FDC3 API Working Group label Oct 16, 2024
@kriswest
Copy link
Contributor

The answer to this is not explicitly stated in the standard to my knowledge (it should be!). However, it consistently says that addContextListener adds a listener - overwriting or replacing an existing listener is never mentioned. Instead, a specific mechanism is provided to remove a listener (Listener.unsubscribe).

Hence, I would go ahead and assume that you can add multiple listeners of the same or overlapping types (i.e. null and a named type) and that all should trigger when a relevant context type is broadcast on the channel or current user channel as appropriate.

I think the answer is different for intent handling, where it only makes sense for a single handler to be present per intent name (context type is not an argument to addIntentListener so a single intent handler should handle all context types accepted) - particularly as only a single handler can relate to the getResult() promis, (which should resolve when the handler has run and has resolved or rejected its promise. However, there is no specified error to return in this case (ResolveError is the most relevant union but doesn't have an error relating to trying to register an intent listener).

Given these two very similar API calls are (and IMHO need to be) handled differently it would make sense for us to clarify this in the documentation of addContextListener (in both DesktopAgent and Channel ) and addIntentListener (in DesktopAgent) - the addition would need to be made both in TS src and docs pages. I think we should also add an error to ResolveError to return if a second intent listener is added for the same intent without first removing the old one.

I'd suggest that this is a clarification worth adding to the 2.3 scope. If you agree with the above and are happy to convert this question into an issue (replace 'Question' in the title with 'Clarify') I think we could agree a PR to resolve quite quickly.

@Roaders
Copy link
Contributor

Roaders commented Oct 16, 2024

I think that it is quite likely that for a given app there might be multiple differnet parts in the app that might want to listen to a single intent being raised. One component might want to actually handle the intent and do something useful but another component might just want to display the context that is currently being handled.

I would say that this should probably be allowed - until @kriswest point about the getResult() implementation. It's going to be non-deterministic if you have multiple handlers all returning an async result. I guess whichever one returns first would be returned but this could lead to random results.

Because of this throwing an error when a second intent handler is registered for the given intent and app seems sensible to me.

@kemerava
Copy link
Contributor Author

Thanks for the reply, @kriswest and @Roaders!

Originally I thought it would make sense to handle both of then with an error, especially since it seems so unintuitive to handle them differently (and, as you, @kriswest, pointed out, for the intent handler, that seems to be the only solution). However, through your examples it makes sense to me to have these behaviors be different and follow your approach.

Definitely agreed on a need for a very specific and clear documentation on this and a separate error. I will edit the title and, whenever I have bandwidth, will work on adding this change (unless someone does it before me)

@kemerava kemerava changed the title Question: What should happen on repeated listener addition for the same contextType/intent Clarify: What should happen on repeated listener addition for the same contextType/intent Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants