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

Missing types for Comlink.finalizer props on objects. #631

Open
ivancuric opened this issue Mar 7, 2023 · 4 comments
Open

Missing types for Comlink.finalizer props on objects. #631

ivancuric opened this issue Mar 7, 2023 · 4 comments

Comments

@ivancuric
Copy link

There is no TypeScript inference for the Comlink.finalizer prop, and it will always throw an error.

Additionally, it's not clear what a "exposed object" is, and on which end it should be called.

If an exposed object has a property [Comlink.finalizer], the property will be invoked as a function when the proxy is being released.

@surma
Copy link
Collaborator

surma commented Mar 8, 2023

“exposed object” is the object that you pass to expose(). So if you do this:

Comlink.expose({
  a: 1,
  b: "lol",
  [Comlink.finalizer]() {
    console.log("Finalized");
  }
});

the finalizer will be called when the proxy on the other side is garbage collected or released.

@ivancuric
Copy link
Author

ivancuric commented Mar 8, 2023

I've never tried defining it inside expose.

If the types are structured in such a way that it only recognizes it inside expose()'s parameters, that means you can't construct an object (or function!) on its own which you'll later expose with a finalizer, without TS complaining.

In this kinda contrived example I've tried exposing a single function and a nested object and multiple questions came up:

  1. Is it ok to call expose multiple times in the same worker?
  2. Is it ok to call wrap on a worker multiple times?
  3. How would I add a finalizer on a function (or object) which was declared outside expose()?
  4. The parameter for the expose() is obj: any kinda implies that you can call it on objects, but the docs show otherwise, when using it for wrapping callbacks. While functions are technically objects in JS, do you think this could be communicated somehow better in JSdocs/naming?
  5. Are functions only supposed to be wrapped with expose when being passed as a callback to another thread?
  6. I'm getting an error Uncaught TypeError: rawValue.apply is not a function at callback when calling an exposed function.
  7. Is objectToExpose considered an exposed object? If so, how would one run finalizers on it?
  8. Is there any use for Comlink.finalizer being exported? If its only use case is internal logic and the only place in which it could be applied is inside expose(), wouldn't it make more sense for a finalizer to be an optional callback argument on it?

@surma
Copy link
Collaborator

surma commented Mar 8, 2023

  1. Is it ok to call expose multiple times in the same worker?

No

  1. Is it ok to call wrap on a worker multiple times?

No

  1. How would I add a finalizer on a function (or object) which was declared outside expose()?
function f() { ... }
f[Comlink.finalizer] = () => ...
Comlink.expose(f);
  1. The parameter for the expose() is obj: any kinda implies that you can call it on objects, but the docs show otherwise, when using it for wrapping callbacks. While functions are technically objects in JS, do you think this could be communicated somehow better in JSdocs/naming?

No, any is literally anything, not just Objects. And that’s true, Comlink — for the must part — doesn’t care what value you are trying to expose.

  1. Are functions only supposed to be wrapped with expose when being passed as a callback to another thread?

expose() is for when you want to expose a value to an endpoint. If you want to pass a callback, you have to use proxy().

  1. I'm getting an error Uncaught TypeError: rawValue.apply is not a function at callback when calling an exposed function.

Can you make a small repro case and file a bug?

  1. Is objectToExpose considered an exposed object? If so, how would one run finalizers on it?

Just like I showed in my previous comment. Or am I misunderstanding you?

  1. Is there any use for Comlink.finalizer being exported? If its only use case is internal logic and the only place in which it could be applied is inside expose(), wouldn't it make more sense for a finalizer to be an optional callback argument on it?

The finalizer isn’t exposed. I used a Symbol() for that reason. It is inaccessible from any other realm. An option would work too, but I think the finalizer is part of the object/type, rather than an option.

@ivancuric
Copy link
Author

ivancuric commented Mar 19, 2023

Ooof, seems that the formatting/reply got a bit jumbled.

For 1. and 2. – this should be mentioned in the docs, and a mechanism should be implemented that would warn the user if they somehow manage to use expose or wrap multiple times on the same worker.

For 6. – the repro is here. The issue is that I used expose to expose a function, and even using expose twice in the same file. It actually worked but it errors as well.

For 8. — having an option on expose which would set a symbol if needed would probably make it a lot easier to work with TypeScript, as it's currently bugging out since expose is basically just a function that triggers a side effect. There is no return value, type assertion or anything else that the user is able to debug, forcing you to declare it inside expose.

It's also not clear if the finalizer can be mutated after being passed to expose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants