Skip to content
This repository was archived by the owner on Mar 24, 2023. It is now read-only.

useSubscription resubscribes on each message received #54

Open
Aposhian opened this issue May 23, 2022 · 6 comments · May be fixed by #55
Open

useSubscription resubscribes on each message received #54

Aposhian opened this issue May 23, 2022 · 6 comments · May be fixed by #55

Comments

@Aposhian
Copy link
Contributor

Aposhian commented May 23, 2022

If I make a very simple component that displays data from a subscription, I have found that it resubscribes every time it receives a new message. This is inefficient, and causes extra latency. My guess is that because the Mqtt client is passed through the context, message receipt changes the client, which then causes the consumers of the mqtt context to rerender. I am looking into this right now.

@Aposhian
Copy link
Contributor Author

Actually, I think the real reason is that useSubscription creates a useEffect that calls subscribe that gets called every time the component gets rerendered. I think the intent is that subscribe is a useCallback, so it should be memoized, but that doesn't actually prevent subsequent calls to subscribe: it just memoizes the passing of the particular topic and options parameters.

Aposhian added a commit to fireflyautomatix/mqtt-react-hooks that referenced this issue May 24, 2022
In order to provide stateful subscriptions that don't get reset with every message receipt, there
needs to be a separation of subscription components

BREAKING CHANGE: useSubscription no longer exists. use createSubscription instead.

fix VictorHAS#54
@VictorHAS
Copy link
Owner

Hey @Aposhian thanks again for the contribution, I was doing some testing on the alpha version and there is a big problem with webpack 5 and react too. what configuration are you using now? I think we can use precompiled-mqtt for temporary solution, what you think? I'll do some tests with you new createSubscription

@Aposhian
Copy link
Contributor Author

I have been getting it to work with Webpack 5 by replacing Create React App with react-app-rewired, and then using the following Webpack overrides:

const webpack = require('webpack');
const path = require('path')

module.exports = function override(config) {
  const fallback = config.resolve.fallback || {};
  Object.assign(fallback, {
    "crypto": require.resolve("crypto-browserify"),
    "stream": require.resolve("stream-browserify"),
    "assert": require.resolve("assert"),
    "http": require.resolve("stream-http"),
    "https": require.resolve("https-browserify"),
    "os": require.resolve("os-browserify"),
    "url": require.resolve("url")
  })
  config.resolve.fallback = fallback;
  config.plugins = (config.plugins || []).concat([
    new webpack.ProvidePlugin({
      process: 'process/browser',
      Buffer: ['buffer', 'Buffer']
    })
  ])
  config.module.rules = (config.module.rules || []).concat([
    {
      test: /\.m?js/,
      resolve: {
        fullySpecified: false
      }
    }
  ])
  return config;
}

@Dyocius
Copy link

Dyocius commented May 31, 2022

Hey @Aposhian thanks again for the contribution, I was doing some testing on the alpha version and there is a big problem with webpack 5 and react too. what configuration are you using now? I think we can use precompiled-mqtt for temporary solution, what you think? I'll do some tests with you new createSubscription

Paho-MQTT seems to not have issues, but it would require switching libraries.

@Aposhian
Copy link
Contributor Author

Aposhian commented May 31, 2022

Paho MQTT (for JavaScript) appears to not be actively maintained though: it hasn't had a commit in 3 years.

@Aposhian
Copy link
Contributor Author

Also, MQTT.js can be made to work just fine with Webpack 5: it just involves readding the polyfills that were removed by default.

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

Successfully merging a pull request may close this issue.

3 participants