KeKs0r / mqtt-react

React container for MQTT
47 stars 31 forks source link

Remove Data[], message to state, modified test #2

Open javi9231 opened 7 years ago

javi9231 commented 7 years ago
KeKs0r commented 7 years ago

@javi9231: It is mainly the question what would be the expected behaviour. To have access to all messages or only the last one.

If you want to modify the behaviour you can also pass in your modified dispatch as own dispatch. I understand that the data will grow over time, but I think having access to all data is a better start.

import { subscribe } from 'mqtt-react';
const customDispatch = function(topic, message, packet) {
    const { state } = this;
    const m = parse(message);

    this.setState({
      message: m
    });
}

export default subscribe({
  topic: '@demo/test',
  dispatch: customDispatch
})
mcollina commented 7 years ago

I fear none it is correct. I think storing any data should be done in some user-provided function. However, we should maintain the state of the subscription, connection etc, as part of the React component.

KeKs0r commented 7 years ago

The user has access to everything on mqtt inside the component, since the mqtt client is exposed to the wrapped component. The connector is currently setting state changes on connection change: https://github.com/KeKs0r/mqtt-react/blob/master/src/connector.js#L51

It might be valuable to also expose this information on the wrapped component, but I would like to see how the component is going to be used in different situations, to see what is useful to expose and what not.

javi9231 commented 7 years ago

@KeKs0r I strongly agree that it is better to save the data, but I would do it in the demo, not in the main library. I would put it here https://github.com/KeKs0r/mqtt-react-demo

You could also implement a data [] handling in the library, to release the data once used. But I think it would limit the performance of the library and the general use.

I want to take this opportunity to congratulate you on the library regards