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.

Sign up for GitHub

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

Jump to bottom

isBrowser check erroneously passes for electron-link bundled node applications #436

Closed
themadtitanmathos opened this issue Apr 1, 2021 · 2 comments · May be fixed by #437
Closed

isBrowser check erroneously passes for electron-link bundled node applications #436

themadtitanmathos opened this issue Apr 1, 2021 · 2 comments · May be fixed by #437
Labels
Area: NodeApi bug stale

Comments

@themadtitanmathos
Copy link

themadtitanmathos commented Apr 1, 2021 •
edited
Loading

azure-devops-node-api version: 10.2.1

Issue Description

global.window gets set in electron-link bundled applications, but global.window.navigator won't exist because it isn't a browser app, it's a node app. This means the current isBrowser check isn't quite enough, and the WebApi constructor will fail while trying to set the userAgent by accessing an undefined window.navigator.userAgent. I think a simple fix would be to also check window.navigator exists when calculating isBrowser.

Expected behaviour

isBrowser should not be true for electron-link (or any electron bundled) node apps.

Actual behaviour

isBrowser is true for electron-link (or possibly other electron bundled) node apps.
@themadtitanmathos themadtitanmathos mentioned this issue Apr 1, 2021
Open
@EzzhevNikita EzzhevNikita linked a pull request Apr 27, 2021 that will close this issue
Bolster isBrowser check for electron-link bundled applications #437
Open
@github-actions
Copy link

This issue has had no activity in 90 days. Please comment if it is not actually stale

@github-actions github-actions bot added the stale label Jul 26, 2021
@github-actions github-actions bot closed this as completed Aug 2, 2021
@Dobby007
Copy link

The issue is still relevant. There is a discussion that is relevant to this: electron/electron#2288

The latest recommendation up there is to use process.versions.hasOwnProperty('electron') in the main process. That's probably what should be done because navigator.userAgent is defined in the renderer so there will be no issues with it (although nobody should invoke API in the renderer).

CC @anatolybolshakov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: NodeApi bug stale
Projects
None yet
Milestone
No milestone
3 participants
@Dobby007 @EzzhevNikita @themadtitanmathos