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

Typescript: message type on SubscriptionCallback<T> based on isRaw #1013

Open
mrjogo opened this issue Dec 26, 2024 · 0 comments
Open

Typescript: message type on SubscriptionCallback<T> based on isRaw #1013

mrjogo opened this issue Dec 26, 2024 · 0 comments
Labels

Comments

@mrjogo
Copy link

mrjogo commented Dec 26, 2024

Description

createSubscription() takes a callback of type SubscriptionCallback<T>, which is (message: MessageType<T> | Buffer) => void;. However, my understanding is message for a given callback will actually only be one of MessageType<T> or Buffer, depending on the value of the option isRaw passed to createSubscription().

Therefore, instead of having message be the union of MessageType<T> | Buffer, there should be two SubscriptionCallback types (or a generic), one with message: MessageType<T> and one with message: Buffer, and the correct one is used depending on the isRaw option value.

The workaround for the current implementation is to add discriminator checks at the top of all callbacks:

if (Buffer.isBuffer(message)) {
  throw new Error("This should never be a Buffer")
}
  • Library Version: 0.27.5
  • ROS Version: Jazzy
  • Platform / OS: Ubuntu 22.04

Steps To Reproduce

node.createSubscription(
      "std_msgs/msg/String",
      "/my_topic",
      {
        isRaw: false,
      },
      (message) => {
        // Gives error:
        // Type 'Buffer<ArrayBufferLike> | String' is not assignable to type 'String'.
        // Property 'data' is missing in type 'Buffer<ArrayBufferLike>' but required in type 'String'.ts(2322)
        const myString: rclnodejs.std_msgs.msg.String = message;
      }
    );

Expected Behavior

No Typescript error.

Actual Behavior

Typescript error, or discriminator required.

@mrjogo mrjogo added the bug label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant