Feature request: Automatic proxy of arguments

[kubijo]

Out of curiosity, why doesn't comlink automatically proxy callbacks out of the box or at least give an opt-in for that?

I have been able to achieve it in the following way…

function $<T>(w: Worker): Remote<T> {
    const remote = comlinkWrap<T>(w);
    return new Proxy<Remote<T>>(remote, {
        get(target, prop, receiver) {
            if (typeof target[prop] === 'function') {
                return new Proxy(target[prop], {
                    apply(target, thisArg, argArray) {
                        // Proxy functions in the argument list as well
                        const proxiedArgs = argArray.map(arg => (typeof arg === 'function' ? comlinkProxy(arg) : arg));
                        return Reflect.apply(target, thisArg, proxiedArgs);
                    },
                });
            }

            return Reflect.get(target, prop, receiver);
        },
    });
}

Am I missing something important here?

[surma]

It does not do it automatically because every callback requires creating a new proxy that holds on to the proxied value, which is basically a memory leak.

If you want to automatically proxy callbacks, you can use transfer handlers. The example for in the README shows how to automatically proxy events and you can use the exact same technique for functions.

[yelouafi]

I was curious about this as well. Mainly because callbacks can’t be sent natively so we’ll always need to proxy them, which seems redundant IMO.

[kubijo]

Yeah, then again... there is a strong chance for memory leaks with callbacks in general. I think I'm gonna in the end rather use some observed triggers in indexeddb (which is part of my whole setup for the project)

[carmelc]

In order to support callbacks in all levels of the objects being transferred I created the following logic, maybe Comlink can provide it as a utility (opt in):

function needsProxy(obj: any): boolean {
  // deeply iterate over the object and check if it includes functions
}

const methodIdentifier = '3#p7C$:comlink-serialized-method-identifier';

const stripMethods = (obj: any): any => {
  // replace any method with `methodIdentifier`
};

const deepMerge = (target: any, source: any) => {
  // recreate the object from the proxy and object with stripped methods
};

Comlink.transferHandlers.set('FUNC', {
  // @ts-expect-error
  canHandle: needsProxy,
  serialize: (obj) => {
    const { port1, port2 } = new MessageChannel();
    Comlink.expose(obj, port1);
    const stripped = stripMethods(obj);
    return [{ stripped, port: port2 }, [port2]];
  },
  deserialize: ({ stripped, port }) => {
    const proxy = Comlink.wrap(port);
    return stripped && stripped !== methodIdentifier
      ? deepMerge(stripped, proxy)
      : proxy;
  },
});

It uses one MessageChannel for all the callbacks nested in a given input object so there is no need to create ports for each callback separately
If you think it is useful I can even contribute it as a PR, so users can use:

import * as Comlink from 'comlink';

Comlink.transferHandlers.set('FUNC', Comlink.autoFunctionsTransferHandler);

What do you think @surma ?