mqttjs / MQTT.js

The MQTT client for Node.js and the browser
Other
8.43k stars 1.41k forks source link

[Bug]: No identifier allowed directly after numeric literal #1890

Open nikhiltekwani09 opened 2 weeks ago

nikhiltekwani09 commented 2 weeks ago

MQTTjs Version

5.7.1

Broker

HiveMQ

Environment

NodeJS

Description

Getting error No identifier allowed directly after numeric literal only on android when hermes is disabled in react native

Screenshot 2024-06-18 at 5 17 14 PM

Error is not reproducible in version 4.3.7 but is reproducible in all versions of series 5

Minimal Reproduction

Just import mqtt and it will start giving error

Debug logs

This error is coming in react native version 0.73.8 only in android and when hermes is disabled

robertsLando commented 2 weeks ago

I'm sorry but I cannot help without more informations like debug logs and/or stack trace.

Be sure to follow docs for react native: https://github.com/mqttjs/MQTT.js#react-native

nikhiltekwani09 commented 2 weeks ago

@robertsLando the problem is that react native is now giving the stack trace and that is the reason I have not shared here. You can also check the screenshot.

Also I would like to know what has changed between series 4 and 5 that the issue started coming.

robertsLando commented 2 weeks ago

https://github.com/mqttjs/MQTT.js/blob/main/CHANGELOG.md

AFAIK there are many users successfully using MQTT 5 with react native so I would check the example I linked above.

Maybe @MaximoLiberata could help you with that error

nikhiltekwani09 commented 2 weeks ago

@robertsLando I am also able to use with react native successfully with hermes enabled in both android and iOS The issue is happening only when hermes is disabled and the platform is android

robertsLando commented 2 weeks ago

@nikhiltekwani09 I have no clue what hermes is as I'm not familiar with Andorid/IOs and react native, I'm sorry

nikhiltekwani09 commented 2 weeks ago

@MaximoLiberata or can someone else help on this?

MaximoLiberata commented 2 weeks ago

Hi everyone. Maybe I can take a look next weekend.

@nikhiltekwani09, Could you create a public repository with a minimal replication of your configuration? Only the needed to replicate the same bug.

nikhiltekwani09 commented 2 weeks ago

@MaximoLiberata I have created an example app using mqtt version 5.7.2 where issue is reproducible Repo Link: https://github.com/nikhiltekwani09/MqttExample

In file https://github.com/nikhiltekwani09/MqttExample/blob/master/App.tsx - I have added a todo to remove a line after which the app loads fine, we need to check why mqtt connect is causing the issue.

MaximoLiberata commented 2 weeks ago

Hi @nikhiltekwani09, I found the error, but it's not a error in the MQTT.JS library. The error stack is the next one:

Screenshot from 2024-06-22 23-56-34

The error was found in the dependency reable-stream in the file's line lib/ours/errors.js #314. This error happens because ReactNative (RN) doesn't have support for BigInt literal 1n, 2n, 3n, ...Nn.

Only adding support for BigInt in the example you can use the mqtt.js library.

I recommend using Expo in your RN projects. RN recommends using Expo instead of vanilla projects ReactNative Get Started. Expo adds some missing tools, dependencies or features that RN doesn't have by default.

nikhiltekwani09 commented 1 week ago

I cannot use expo, as I serve my code as a sdk, my client can have expo, hermes or no hermes, all depends on them. So my sdk should be able to support all variants.

Also even if I remove that line from node_modules, the error is still reproducible, can you tell me how issue got fixed for you, how did you add support for BigInt @MaximoLiberata ?

nikhiltekwani09 commented 4 days ago

Hi @MaximoLiberata any updates on this please?

MaximoLiberata commented 4 days ago

Hi @nikhiltekwani09, I've been busy these days, I'll replicate my steps to solve the problem and post them this weekend.

MaximoLiberata commented 1 hour ago

I replicate the same steps I posted before (https://github.com/mqttjs/MQTT.js/issues/1890#issuecomment-2185408405) and worked. In that occasion I only commented that part of code to start up the application and test with mosquitto mqtt server. I haven't found a solution to fix the dependency readable-stream using plugins like babel yet.

My mosquitto configuration was the next:

import {connect} from 'mqtt';

const client = connect({
  host: 'test.mosquitto.org',
  port: 8081,
  protocol: 'wss',
  protocolVersion: 4
})
.on('connect', function () {

  console.log('CONNECTION_CONNECTED')

  client.subscribe('#', function (err) {
    if (err) {
      console.error('SUBCRIPTION_ERROR', err);
    }
    else {
      console.log('SUBCRIPTION_SUCCESS');
    }
  });

})
.on('message', function (topic, message) {
  console.log('TOPIC', topic); 
})
.on('error', function (error) {
  console.error('CONNECTION_ERROR', error);
});

On the other hand, I make a solution for the dependency in this fork redeable-stream-bigint. To implement it you need to do the next:

  1. Download the repository redeable-stream-bigint and change to the branch react-native-bigint
  2. Install big-integer in your package.json
  3. Change lib folder of the dependency readable-stream in your node_modules folder with the lib folder of the repository redeable-stream-bigint

@robertsLando Do you think this is a good solution for the dependency?

MaximoLiberata commented 1 hour ago

I'm testing the example MqttExample with node v20.15.0 and emulating with Pixel 3a API 34