-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 compatibility with txiki.js #1895
Conversation
Problem is that NodeJS is introducing Websocket support so when that will happen that check would wrongly assume the env to be node and not browser. I have thought many times how this could be improved but I never found a better way to do this, if you find one please tell me or open a PR :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix lint issue npm run lint-fix
Oops I would need to git force, I committed with my wrong email address. |
txiki.js is compatible with most of the browser api, but it does not have the document variable.
What the maintainer of txiki.js propose:
|
Head branch was pushed to by a user without write access
@EmixamPP Thanks for your link, problem is that I think we need to postpone this till all our envs supports |
Sorry for the confusion, but is merging the current solution to support |
@EmixamPP It is ok, what I meant is just that method cannot be used (yet) as I don't think it would support ALL our cases and old frameworks version that may don't have |
Thanks for your extremely fast review and merge! |
This reverts commit 37b08c9. Not special support for txiki.js is required thanks to forceNativeWebSocket client option
* Revert "fix(browser): add `navigator` polifilly for wechat mini (#1796)" This reverts commit c26908a. since the polyfill hardcode navigator, it is impossible to determine the userAgent at runtime * doc(README): update WeChat instructions * feat: add forceNativeWebSocket client option * Revert "feat: add compatibility with txiki.js (#1895)" This reverts commit 37b08c9. Not special support for txiki.js is required thanks to forceNativeWebSocket client option * style: unify import name IS_BROWSER -> isBrowser * fixup! feat: add forceNativeWebSocket client option style: fix lint * fixup! fixup! feat: add forceNativeWebSocket client option 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
txiki.js is a small JavaScript runtime compatible with most of the browser api; very nice for small embedded devices.
However,
MQTT.js
wrongly assume aNode.js
compatibility because there is nodocument
variable in the global space. Although, if it was using the browser ws, the lib would be compatible with it.After discussion saghul/txiki.js#557 with the creator of
txiki.js
, we both think that it should be the job to add the compatibility.That's what I've added in the present PR.
Small remark, I also think that, in the future, it may be good to revisit the way
MQTT.js
determine if it can use native websocket orNode.js
one through thews
module. Like checking ifWebSocket
exists in the global space, instead of trying to determine the runtime name or whatsoever sophisticated tricks.