Skip to content

Conversation

@cmoniz-ocean
Copy link
Contributor

@cmoniz-ocean cmoniz-ocean commented May 17, 2024

Fixes error

ReferenceError: navigator is not defined
    at isStandardBrowserEnv (node_modules/mqtt/src/lib/is-browser.ts:7:4)

(error is thrown in electron 12.2.3 and electron 30.0.6)

6a03d29#commitcomment-142114121

Fixes electron 12.2.3, which doesn't have navigator defined

mqttjs@6a03d29#commitcomment-142114121
@cmoniz-ocean cmoniz-ocean changed the title Update is-browser.ts to account for undefined navigator fix: Update is-browser.ts to account for undefined navigator May 17, 2024
@cmoniz-ocean cmoniz-ocean changed the title fix: Update is-browser.ts to account for undefined navigator fix: Update is-browser.ts to account for undefined navigator May 17, 2024
cmoniz-ocean referenced this pull request May 17, 2024
* fix(electron): detect electron env

* fix(electron): cleanup code

* fix: fixed wrong operator

* fix(electron): improved code and add some comments

* Update src/lib/is-browser.ts

Co-authored-by: Daniel Lando <[email protected]>

* fix: typo and lint

---------

Co-authored-by: Daniel Lando <[email protected]>
@robertsLando robertsLando changed the title fix: Update is-browser.ts to account for undefined navigator fix: update is-browser.ts to account for undefined navigator May 17, 2024
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.

Did you verified this is actually working on Electron?

// check if we are in electron `renderer`
const electronRenderCheck =
(typeof navigator !== "undefined") &&
navigator?.userAgent?.toLowerCase().indexOf(' electron/') > -1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
navigator?.userAgent?.toLowerCase().indexOf(' electron/') > -1
navigator.userAgent?.toLowerCase().indexOf(' electron/') > -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@robertsLando I've run npm run lint-fix and made the above changes #1868

Additionally, this has been npm run builded, npm packed, and then npm installed to my local electron 30 project and looks like it's working.

@robertsLando robertsLando changed the title fix: update is-browser.ts to account for undefined navigator fix: update is-browser.ts to account undefined navigator May 17, 2024
@robertsLando
Copy link
Member

Also pleasew fix lint issues with npm run lint-fix

@axi92
Copy link
Contributor

axi92 commented May 17, 2024

I am happy to verify the changes later as well!

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.

4 participants