awslabs / aws-crt-nodejs

NodeJS bindings for the AWS Common Runtime.
Apache License 2.0
38 stars 24 forks source link

Node and Browser versions of the `iot.AwsIotMqttConnectionConfigBuilder` do not provide the same API #426

Closed massi-ang closed 1 year ago

massi-ang commented 1 year ago

The iot.AwsIotMqttConnectionConfigBuilder module has 2 different APIs depending if used with the browser or with node. This does not allow me to write code that works the same in both environments and does not play well with VS Code intellisense/autocompletion which select the node version.

While implementation should differ depending , the APIs should be the same.

xiazhvera commented 1 year ago

As the browser and the node library have different features/functionalities, it is impossible to make them the same. There will always be some gap between the two APIs. However, we will try our best to unify them. We've added the task on our road map. Thank you to bring this up!

massi-ang commented 1 year ago

For the intellisense issue, one solution is to explicitly import the specific library, in case I know that the code is for the browser or node respectively. Eg:

import { mqtt, iot } from "aws-crt/dist.browser/browser";

provides the right intellisense suggestions.

xiazhvera commented 1 year ago

Thank you for sharing the intellisense solution!

xiazhvera commented 1 year ago

Hi @massi-ang, after reviewing the difference in the APIs. We figured there some missing API we can fix. As an example: new_websocket_builder was missing in browser library. Other than that, there is little we can do because of the difference behind browser and node connection. Is there any specific interface you would like us to improve? Let me know if you have any suggestions.

massi-ang commented 1 year ago

Hi, I understand that the native implementation supports some configurations that are not available in the browser implementation, but the common subset of the methods between the two implementations should have the same signatures.

xiazhvera commented 1 year ago

The main difference between Node library and Browser library is the way they setup the websocket. To satisfied the async design on browser side, we have to change how the websocket setup, which causes the API difference.

Besides the websocket configurations, there is also some mismatch on parameters. For example, with_socket_options will take different SocketOptions as they are implemented differently on Node side and browser side. We do not have a way to unify them for now.

As for now, we will add new_builder_for_websocket for API compatibility with the native version. Here is a table of the current mismatched APIs:

Node Browser Status Note
new_mtls_builder_from_path -- Invalid Browser not support
new_websocket_builder new_builder_for_websocket Mismatch This function in Native directly calls new_with_websockets.
new_mtls_windows_cert_store_path_builder -- Invalid Browser not support
new_mtls_pkcs11_builder -- Invalid Browser not support
with_certificate_authority_from_path -- Invalid Browser not support
with_certificate_authority -- Invalid Browser not support
with_protocol_operation_timeout_ms -- Invalid Browser not support (Mqtt.js does not have a operation timeout settings for mqtt3)
with_will with_will Different parameters  
with_socket_options with_socket_options Different parameters
-- with_websocket_headers Invalid The Node library implement the websocket configs in a different way.
-- with_credential_provider Invalid The Node library implements the credential provider in a different way.
massi-ang commented 1 year ago

Thank you.

xiazhvera commented 1 year ago

Closing issue here. Feel free to reopen it if you have further consideration related to the API.