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

Drop connect #1648

Open
robertsLando opened this issue Jul 25, 2023 · 5 comments
Open

Drop connect #1648

robertsLando opened this issue Jul 25, 2023 · 5 comments

Comments

@robertsLando
Copy link
Member

robertsLando commented Jul 25, 2023

The connect logic should be moved on MqttClient constructor or a dedicated private method like setup.

ATM it is used to parse provided options and validate them, also creates a wrapper to use the correct streamBuilder based on provided protocol and support for handling servers option.

Users should just create the client instance by passing the options and then calling client.connect to connect later

Of course this would be a BREAKING CHANGE. My plan is to do all major breaking changes in v6. v6 will not be back compatible with v5 anymore but will try to fix all the problems this library has

cc @vishnureddy17 @BertKleewein opinions about this?

@vishnureddy17
Copy link
Member

vishnureddy17 commented Jul 25, 2023

If this will be included in V6, then I think we should consider making it a "bring your own connection" library. This would greatly reduce the number of options we have to deal with and allow custom connection implementations (e.g., WeChat) without polluting the project code.

However, this makes things like reconnecting tricky. We might have to push that responsibility onto the user

In my opinion, instantiation of the client should not do any I/O. It should just validate options, and the user has to call a connect() method to start the first I/O (sending the CONNECT packet)

@robertsLando
Copy link
Member Author

robertsLando commented Jul 25, 2023

However, this makes things like reconnecting tricky. We might have to push that responsibility onto the user

Nope, that should still be handled by client itself IMO. We just should move the logic there

This would greatly reduce the number of options we have to deal with and allow custom connection implementations (e.g., WeChat) without polluting the project code.

Yes but where should be put actual implementations? Because users will ask for them, we should have a place, maybe another repo or gists or I dunno whatelse

@vishnureddy17
Copy link
Member

vishnureddy17 commented Jul 25, 2023

Yes but where should be put actual implementations? Because users will ask for them, we should have a place, maybe another repo or gists or I dunno whatelse

Perhaps we can have the client be able to create the most common connections (tls, tcp, websockets), making sure that the options that are specific to the connection have a clear separation from the MQTT protocol options.

For custom connections like WeChat, I do not think we should be responsible for helping users there. If we allow "bring your own connection" we would be responsible for documenting and supporting the "bring your own connection" functionality, but WeChat would be responsible for their specific implementation details. They can still use V5 if they don't want that

@robertsLando
Copy link
Member Author

robertsLando commented Jul 25, 2023

For custom connections like WeChat, I do not think we should be responsible for helping users there. If we allow "bring your own connection" we would be responsible for documenting and supporting the "bring your own connection" functionality, but WeChat would be responsible for their specific implementation details. They can always use V5 if they don't want that

It's ok for me!

P.S: V5 is coming: https://github.com/mqttjs/MQTT.js/actions/runs/5658865891 👀

@vishnureddy17
Copy link
Member

Yay! Thanks for the hard work on this :)

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

No branches or pull requests

2 participants