@axi92 could you take a look at this to be sure it doesn't break you?

Ref: #1856

https://www.electronjs.org/docs/latest/api/process#processtype-readonly

@axi92 could you take a look at this to be sure it doesn't break you?

Ref: #1856

I can check later today. Thanks for mentioning me!

Maybe somebody could implement a test with playwright? https://www.electronjs.org/docs/latest/tutorial/automated-testing#using-playwright
I have never done a test with the electron context but maybe it's possible to have a fully automated test that checks if mqtt is still working with the latest electron version.

@axi92 Here we have wtr (who uses playwright under the hoods) but I have no clue how to configure it with electron... IMO the easier way is to simply create a super simple electron app that connects to a local mqtt server

npm run build returns errors.
@GuoSirius were you able to build your code?

$ npm run build

> [email protected] build
> npm run build:ts && npm run build:browser


> [email protected] build:ts
> rimraf build/ && tsc -p tsconfig.build.json

src/lib/is-browser.ts:5:16 - error TS2339: Property 'type' does not exist on type 'Process'.

5    if (process.type === 'renderer') return true
                 ~~~~

src/lib/is-browser.ts:6:28 - error TS2339: Property 'electron' does not exist on type 'Process'.

6    else if (typeof process.electron !== 'undefined') return false
                             ~~~~~~~~


Found 2 errors in the same file, starting at: src/lib/is-browser.ts:5

@axi92 It's all right now

I checked the current branch and it breaks for me.
It uses ws instead of mqtt in the browser context of electron.

What was your vision of an simple electron app?
An automated test or a test that is manually executed in an environment with a display?

@axi92 An automated test of course with a dedicated GH workflow like for browser tests

@robertsLando I created #1883 as a poc that electron tests are possible.

In fact, this test should not be placed here for judgment.

Because the rendering process is also a browser environment, since it is a browser environment, it should go the browser logic.

Otherwise, the ws module may be mistakenly loaded in the browser environment of the rendering process, and the ws module cannot run in the browser environment.

If you do need to use the mqtt protocol in an electron application, it is recommended to put the connection establishment logic in the main process, and then use the interprocess communication mechanism to interact between the row rendering process and the main process.

The rendering process must have a window and a document, but not necessarily a Node.js environment, depending on the configuration at instantiation time.

With the main rendering process's responsibilities clearly divided, the browser environment's detection mechanism can maintain the previous standard judgment logic.

If you do need to use the mqtt protocol in an electron application, it is recommended to put the connection establishment logic in the main process, and then use the interprocess communication mechanism to interact between the row rendering process and the main process.

I'm not an electron user but I think it makes sense. Also mqtt-explorer (an electron application that uses mqtt) seems to do it this way

https://github.com/thomasnordquist/MQTT-Explorer/tree/f539e03c7ebc44ab274159ba370b14fcd269b414?tab=readme-ov-file#develop

If you use electron in its best practice mode that is the case, yes I agree with you.

On the other hand if you do ignore that on purpose because you choose to, I don't know if it is a good option to deny the user that option.

The workaround we used before there was electron support was to patch mqtt after npm i

@axi92 I'm not blocking this, I just would like to be sure it works for all scenarios :)

The Electron main process owns all Node environments and can establish connections according to the logic of non-browser environments.

The rendering process belongs to the browser environment and should follow the logic of the browser environment to establish the connection.

Just because the rendering process has more process, Node, and other accessible environments than the normal browser environment, it should not be attributed to the Node environment.

In this cross-end application, it is up to the user to decide which way to establish the connection. According to the needs, the connection can be established in the main process or the rendering process, rather than the application library to force detection to determine the protocol.

According to the above convention, the is-browser.ts file is purely used to detect the browser environment, regardless of whether there are other factors, such as the main process or the rendering process.

@GuoSirius I understand, so is there a way to allow both you and @axi92 being able to use mqtt with electron as you want? I don't want to break anyone I just would like to allow everybody using this lib as they prefer, maybe a way to override the is_browser check could work? with an env var maybe. Giving that your solution seems the most correct one and I agree with you we could use yours to detect the electron rendering process and consider it browser and in order to also allow @axi92 using it like he want add an env var to allow users forcing the value of is_browser. What do you think?

cc @axi92

Override is a good option! Either with options or an ENV var as long as it is possible to override the check I am fine with it 👍

Ok so @GuoSirius would you like to add the override with the env var to this PR please?

Could this be closed now that we have forceNativeWebSocket option?

Any updates for this issue?

forceNativeWebSocket seems can't solve this.

I can not connect to broker with tcp or mqtt protocol in a node integrated renderer process.

When I specify to use the TPC or MQTT protocol for connection, it automatically adjusts to use the WS protocol instead. It doesn't seem reasonable.

In my opinion, if the caller specifies a protocol, the library should simply use the specified protocol to establish connection. If the current environment doesn't support it, just throw an exception for troubleshooting.

Environment detection may be useful for some users, but this mechanism cannot achieve 100% accuracy. Therefore, I believe that an option is needed to tell the library directly bypass environment detection and work solely in the way specified by caller.

@ericlogic forceNativeWebsockets is used to force MQTTjs to use browser's Websocket instead of the ws module, your problem seems related due to MQTTjs detecting your env as a browser when it is not, would you like to submit a PR to fix this?

@robertsLando Yes, you are right.

I did some research and it's a little complicated.

First, isStandardBrowserEnv method isn't work as expected. It is executed when is-browser.ts is imported, at that time, "process" object is an empty object. But it works OK at runtime.

After that, when I walked around the isBrower check, The library throwed an Error.

Uncaught Error: Node.js net module is not supported by JSPM core outside of Node.js
    at Object.Me (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:13438)
    at Object.pS (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:13484)
    at t.s [as streamBuilder] (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:13752)
    at t.connect (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:11531)
    at new t (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:11511)
    at Object.Pg (webpack-internal:///./node_modules/mqtt/dist/mqtt.esm.js:13754)

The error is throwed in the mqtt.esm.js but I can't find any related code in source code. It seem that there is a lot of work to do.