Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Electron browser environment check #1877

Open
GuoSirius opened this issue Jun 4, 2024 · 5 comments · May be fixed by #1879
Open

[Bug]: Electron browser environment check #1877

GuoSirius opened this issue Jun 4, 2024 · 5 comments · May be fixed by #1879
Labels
bug electron Electron JS related issues

Comments

@GuoSirius
Copy link

GuoSirius commented Jun 4, 2024

MQTTjs Version

5.7.0

Broker

RabbitMQ

Environment

Electron Renderer & Browser

Description

Electron Renderer process still exist electron keyword, cannot check browser environment

Minimal Reproduction

image

Debug logs

image

@GuoSirius GuoSirius added the bug label Jun 4, 2024
@GuoSirius GuoSirius changed the title [Bug]: [Bug]: browser environment check Jun 4, 2024
@robertsLando
Copy link
Member

@GuoSirius Could you submit a PR to fix the issue please?

@GuoSirius
Copy link
Author

@GuoSirius Could you submit a PR to fix the issue please?

if (process.type === 'renderer') {
  console.log('current  Electron Renderer');
} else if (typeof process.electron !== 'undefined') {
  console.log('current Electron Main');
}

@GuoSirius
Copy link
Author

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.

@GuoSirius
Copy link
Author

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.

@robertsLando
Copy link
Member

robertsLando commented Jun 12, 2024

I answer to you in the PR

@robertsLando robertsLando changed the title [Bug]: browser environment check [Bug]: Electron browser environment check Jun 24, 2024
@robertsLando robertsLando added the electron Electron JS related issues label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug electron Electron JS related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants