0

I'm new to React and I've created a simple hook to allow some components to subscribe to MQTT messages through a global connection to the broker stored in a context. I'm hoping to encapsulate the MQTT bits into this one file and allow my low-level components to read values from it and publish to it while letting the middle layers of the component tree ignore MQTT and do the layout. I'm open to suggestions on the design of this app if redux would be better or anything like that.

I'm experiencing a race condition where my values are only show on some page refreshes but not others. Confusingly, useEffect is not being called more than twice early on and I was expecting it to be called on every page render. Perhaps that's not occurring with each update to the mqtt incoming on('message'). I'd like it to respond when a message comes in.

Furthermore, annoyingly my mqtt.connect is called about 4 times when I run this, I think because it's trying again so quickly before it actually connects. The if (client == null) has not changed yet.

src/App.tsx:

import Welcome from "./components/Welcome"
import ReadOnlyWidget from "./components/ReadOnlyWidget"
import { useMqtt, MqttProvider } from "./MqttContext"

const url = 'ws://10.0.200.10:9001'

export default function App() {
  return (
    <MqttProvider brokerUrl={url}>
        <ReadOnlyWidget topic="/sample/tower-mill/drive/feed" field="feed_distance" />
        <ReadOnlyWidget topic="/sample/tower-mill/drive/feed" field="feed_velocity" />
    </MqttProvider>
  );
}

src/MqttContext.tsx:

import React from "react"
import mqtt from 'precompiled-mqtt'
import _ from 'lodash'

export const MqttContext = React.createContext(null)

export const MqttProvider = ({ brokerUrl, children }) => {
  const [client, setClient] = React.useState(null)
  const [messages, setMessages] = React.useState({})

  if (client == null) {
    const newClient = mqtt.connect(brokerUrl)

    newClient.on('connect', () => {
      console.log("new client connected")
    })

    newClient.on('disconnect', () => {
        console.log('new client disconnected')
        setClient(null)
    })

    newClient.on('message', (topic, message, packet) => {
      const json = JSON.parse(new TextDecoder("utf-8").decode(message))
      console.log(json)
      setMessages(_.set(messages, topic, json))
    })

    setClient(newClient)
  }

  return (
    <MqttContext.Provider value={{ client, messages }}>
      {children}
    </MqttContext.Provider>
  )
}

export const useMqtt = ({topic, field}) => {
  const mqttContext = React.useContext(MqttContext)
  const [value, setValue] = React.useState(null)
  mqttContext.client.subscribe(topic)
  React.useEffect(() => {
    console.log("use effect")
    setValue(_.get(mqttContext.messages, [topic, field]))
  })
  return value
}

src/components/ReadOnlyWidget.tsx:

import React from 'react';
import { useMqtt } from "../MqttContext"

export default (props) => {
    const value = useMqtt({topic: props.topic, field: props.field})

    return (
      <p>{props.topic} {props.field} {value}</p>
    )
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
mmachenry
  • 1,773
  • 3
  • 22
  • 38

2 Answers2

1

Your useEffect is missing a dependency array

React.useEffect(() => {
    console.log("use effect")
    setValue(_.get(mqttContext.messages, [topic, field]))
  }, [mqttContext, topic, field, messages]);

useEffect does not get called on every render, but rather execute the callback function (effect) passed to it whenever the value of a variable within the dependency array changes. An empty dependency array will run exactly once (twice in strict mode) when the component mounts, and never again. You said you want it to be called every time a message comes in, so if you take your messages state variable from MqttContext.tsx would be ideal. Since hooks are functions, however, it's not as simple as passing that as a parameter (since that only gets the initial value). I would instead consider migrating your connection handling into the hook and return multiple values, ie

const thingsToReturn = {
    value,
    messages
}
return thingsToReturn;

and grab them wherever needed by

const {
    value,
    messages
} = useMqtt("your params here");
Eric Webb
  • 361
  • 3
  • 10
  • It seems like the documentation says that the extra argument of a list of objects passed to useEffect is a list of restrictions where if they useEffect would be called and any of the objects is unchanged it won't call it. Not that useEffect triggers if they change. I tried this and it did not have any effect either. Are you sure you have the arguments right? Did that work for you? – mmachenry Oct 05 '22 at 05:23
  • Looking at the example in "Using the Effect Hook" https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects "You can tell React to skip applying an effect if certain values haven’t changed between re-renders. To do so, pass an array as an optional second argument to useEffect." If there is no dependency array, there's nothing passed to the effect that can change, and by extension, the effect won't be called. For that reason, you for sure want the messages variable in that array, since it's changed in your on("message") and you want the effect to fire on message – Eric Webb Oct 05 '22 at 12:12
  • Admittedly I haven't tried the solution (I haven't worked with Mqtt), I just did my best to look at what your app is doing (useEffects work the same way regardless of what they need to do). Since messages is changed on every message recieved (state set), when the component rerenders, any useEffect with messages as a dependency will fire as well @mmachenry – Eric Webb Oct 05 '22 at 12:19
  • Yes I'm reading the same docs. I read exactly that line "You can tell React to skip applying an effect if certain values haven’t changed between re-renders. To do so, pass an array as an optional second argument to useEffect:" To me, that sounds like it's only going to cause the effect to not run in the event that it would run. Which makes sense because it's under optimizations in the docs. But my entire problem is that this hook is not running when I need it to be running. I think that your solution only prevents the hook from running more. – mmachenry Oct 05 '22 at 13:45
  • A useEffect with no dependency array will run on *every* component rerender, which looking at your example, is not what you want. Your effect will cause a state rerender (setValue), so your effect hook will need a dependency array to prevent it from running into an infinite loop as seen https://stackoverflow.com/questions/53070970/infinite-loop-in-useeffect The dependency array will improve not only performance, but also make it perform in an expected manner. Did moving newClient.on('message'...) into your useMqtt hook (and with it your [messages, setMessages]) throw any log outside useEffect – Eric Webb Oct 05 '22 at 14:51
0

The answer to this was a actually that no changes were being registered by React due to referential equality vs. structural equality.

setMessages(_.set(messages, topic, json))

We creating a reference identical to the original messages when there was no update. The "equivalent" expression

{...messages, [topic]: json}

Creates a new reference every time. This causes the useEffect to run constantly and always eventually show something regardless of the rendering race conditiion.

However, this is not a create use of useEffect and I have rewritten the hook to add a subscription system to subscribe to the on message updates and directly call into the hook's local state instead. But the above does fix the code and cause what I expected to happen to happen.

mmachenry
  • 1,773
  • 3
  • 22
  • 38