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

feat: add forceNativeWebSocket client option #1910

Merged
merged 7 commits into from
Jul 26, 2024

Conversation

EmixamPP
Copy link
Contributor

@EmixamPP EmixamPP commented Jul 25, 2024

Solve #1905 (and could also partially solve #1900).

  • add a forceNativeWebSocket option to the client, if set to true, by-pass the isBrowser detection by src/lib/is-browser.ts. This allows to easily solve the ws does not work in the browser exception for non-standards env/runtime.
  • remove the build option navigator of the polyfill esbuild-plugin-polyfill-node, since it defines an arbitrary value for the navigator variable, breaking many usages at runtime of that variable, like navigator.userAgent or navigator.product when using the .esm dist, which then also break the env detection. Instructions for WeChat have been updated.
  • Remove auto-detection for txiki.js, which is now well-supported if forceNativeWebSocket is used (no need to complicate the code for niche use case).
  • unify the import name for IS_BROWSER to isBrowser (does not facilitate the task when debugging browser detection).

…js#1796)"

This reverts commit c26908a.
since the polyfill hardcode navigator, it is impossible to determine the userAgent at runtime
This reverts commit 37b08c9.
Not special support for txiki.js is required thanks to forceNativeWebSocket client option
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove also test-store folder

README.md Outdated Show resolved Hide resolved
src/lib/connect/index.ts Outdated Show resolved Hide resolved
src/lib/client.ts Outdated Show resolved Hide resolved
chore: remove test_store folder pushed
refactor: load protocols only once
refactor: use forceNativeWebSocket only for ws choice
doc(README): typo + update forceNativeWebSocket behaviour description
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@robertsLando robertsLando changed the title feat: forceNativeWebSocket client option feat: add forceNativeWebSocket client option Jul 26, 2024
@robertsLando robertsLando merged commit 103d172 into mqttjs:main Jul 26, 2024
6 checks passed
@robertsLando
Copy link
Member

@EmixamPP Version 5.9.0 published 🚀

@EmixamPP
Copy link
Contributor Author

Fast! Thank you!

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

Successfully merging this pull request may close these issues.

2 participants