DamonOehlman / detect-browser

Unpack a browser type and version from the useragent string
MIT License
689 stars 102 forks source link

Add electron as detected browser #184

Open syabro opened 3 months ago

syabro commented 3 months ago

I'm building a webapp and want to distinguish Electron from Chrome in posthog, which is using detect-browser :(

DamonOehlman commented 2 months ago

@syabro Do you have ability to add some custom code of your own in there to work out whether it's electron? Or limited to what posthog (I'm not that familiar with their service) allows you to determine?

It looks like this code snippet is the most accepted way to detect electron.

While things like this can be integrated into this library, I've been hesitant to do that as I haven't wanted to add additional weight for folks that don't need that functionality. Might be a way around that though with targeting specific imports, though that might not work in the posthog case.

syabro commented 2 months ago

@DamonOehlman thanks for the reply!

I dont' have access to posthog's scripts. So they do the detection by using detect-browser. I guess a lot of analytics scripts do the same.

I've been hesitant to do that as I haven't wanted to add additional weight for folks that don't need that functionality

I understand, but I believe it’s crucial to distinguish Electron from Chrome rather than for example Brave from Chrome. It’s no longer 2005, and we don’t need to optimize for different engines.

For me, it’s important because I have an Electron application that uses the website to display content. If I can detect Electron, I know someone is using my app, not just browsing the site.

DamonOehlman commented 2 months ago

For me, it’s important because I have an Electron application that uses the website to display content. If I can detect Electron, I know someone is using my app, not just browsing the site.

Totally understand - good to have that info. I've just signed up for posthog to try and understand how things might be used in their platform.

Having a quick think about this and going back through my own code I'm actually wondering whether electron should be detected as a different type at a top level rather than a browser variant. Here is the current list of browser "types" for reference:

export type DetectedInfoType =
  | 'browser'
  | 'node'
  | 'bot-device'
  | 'bot'
  | 'react-native';

It might not be that electron should be a type here, but rather something like app-container (or desktop-wrapper, etc) and electron is then the variant. I know there used to be a bit of competition in that space, but maybe electron has emerged the clear choice now 🤷🏻

syabro commented 2 months ago

@DamonOehlman from my perspective it's a browser, not a platform. Technically it's chromium inside so if it was my call I would go with adding a new browser. But up to you 🙂

DamonOehlman commented 2 months ago

Cool cool - I'll have a bit of a think about it all and might even reach out to the posthog folks to see how things work in their systems also.

syabro commented 2 months ago

https://github.com/umami-software/umami/blob/master/package.json#L87C6-L87C20 https://github.com/umami-software/umami/blob/master/src/lib/detect.ts#L134

For the record: Umami uses it as well

const browser = browserName(userAgent);
DamonOehlman commented 2 months ago

@syabro Thanks for the heads up - definitely sounds like it's something that needs to be dealt with for folks.

I've been pretty snowed under this week but hoping to get to looking into it next week :)

syabro commented 2 months ago

@DamonOehlman I think adding electron detection as a separate browser it wouldn't affect 99.99% users since nobody uses it on purpose. But in this case tracking it will be great :)