useHostStyles internals override each other

[Avcharov]

Describe the bug
useHostStyles internally uses useHostStyleVariables() and useHostFonts()

Inside useHostStyleVariables and useHostFonts are assigning different callback to app.onhostcontextchanged which causes overriding while using both of methods together inside useHostStyles:

// useHostFonts
app.onhostcontextchanged = (params) => {
  if (params.styles?.css?.fonts) {
    applyHostFonts(params.styles.css.fonts);
  }
};

// useHostStyleVariables
app.onhostcontextchanged = (params) => {
  if (params.theme) {
    applyDocumentTheme(params.theme);
  }
  if (params.styles?.variables) {
    applyHostStyleVariables(params.styles.variables);
  }
};

// useHostStyles
export function useHostStyles(
  app: App | null,
  initialContext?: McpUiHostContext | null,
): void {
  useHostStyleVariables(app, initialContext);
  useHostFonts(app, initialContext);
}

Overriding happens due to onhostcontextchanged implementation which is based on setNotificationHandler.
typescript-sdk mentions that previous handlers will be overwritten:

  /**
     * Registers a handler to invoke when this protocol object receives a notification with the given method.
     *
     * Note that this will replace any previous notification handler for the same method.
     */
    setNotificationHandler<M extends NotificationMethod>(
        method: M,
        handler: (notification: NotificationTypeMap[M]) => void | Promise<void>
    )

To Reproduce
Steps to reproduce the behavior:
Try to track host style variables changes after using useHostStyles, it wont we tracked

Expected behavior
useHostStyles() works w/o overriding onhostcontextchanged callbacks

Proposal

1. Instead of reusing useHostStyleVariables and useHostFonts inside useHostStyles just put all needed code into single callback and assign it to onhostcontextchanged.

OR

2. Draft PR: feat: implement subscribe/unsubscribe methods for multiple event handlers #552
   Implement subscribe(eventName) / unsubscribe(eventName) methods for multiple handlers. Then useHostStyleVariables and useHostFonts would look like:

//useHostFonts
const unsubscribe = app.subscribe("hostcontextchanged", (params) => {
  if (params.styles?.css?.fonts) {
    applyHostFonts(params.styles.css.fonts);
  }
});

return unsubscribe;

//useHostStyleVariables
const unsubscribe = app.subscribe("hostcontextchanged", (params) => {
  if (params.theme) {
    applyDocumentTheme(params.theme);
  }
  if (params.styles?.variables) {
    applyHostStyleVariables(params.styles.variables);
  }
});

return unsubscribe;

[Avcharov]

@ochafik Can you kindly take a quick look?

[ochafik]

Hey @Avcharov, thanks for reporting this sharp edge / proposing solutions!

I wonder how we could do 1. (put all needed code into single callback and assign it to onhostcontextchanged.).

A low-footprint alternative could be to expose a getter for the on* handlers, so users could compose const previousHandler = app.onhandler; app.onhandler = x => { ... ; previousHandler(x); }

cc/ @jonathanhefner wdyt?

[Avcharov]

Hi, @ochafik thanks for feedback :)

I see some cases when prev implementation can fail.

If we have 3 chained handlers C -> B -> A and B will be unsubscribed then we will loose all handlers after B, so C will be lost.
Thats why i did proposal for subscribe methods as each handler will be independent.

P.S.

By put all needed code into single callback and assign it to onhostcontextchanged. i mean this:

//useHostStyles
   app.onhostcontextchanged = (params) => {
      if (params.styles?.css?.fonts) {
        applyHostFonts(params.styles.css.fonts);
      }
      if (params.theme) {
        applyDocumentTheme(params.theme);
      }
      if (params.styles?.variables) {
        applyHostStyleVariables(params.styles.variables);
      }
    };

[jonathanhefner]

By put all needed code into single callback and assign it to onhostcontextchanged. i mean this:

I actually tend to favor this approach. My guess is that, in practice, people are only interested in useHostStyles. I might even suggest deprecating useHostStyleVariables and useHostFonts so that people don't accidentally shoot themselves in the foot.