fs-shim#isElectron() is not a robust evaluation of whether the app is truly an Electron app

[dp-kb]

https://github.com/orbitdb/orbit-db/blob/32cc9a9b009d0d8543f81a6bab96ad9bcccd3088/src/fs-shim.js#L4

The fs-shim is incorrectly interpreting my browser app as an Electron app in this method. As a result it is trying to run require("fs") which is crashing the app with: ReferenceError: require is not defined.

The logic that is specifically failing is this:

if (typeof window !== 'undefined' && typeof window.process === 'object') {
    return true
}

My window.process is set. It's a semi-common polyfill to make Angular 6+ apps work with libraries that were not written for universal use. Unfortunately, 3box is one of those libraries, and because 3box has a dependency on orbit-db, it becomes somewhat of a Catch 22.

[phillmac]

What do you recomend? We type check require() exists? Afaik require is a shim that is defined in some browser bundlers making this a rather thorny problem

[phillmac]

IRRC there was some talk of reversing the depenancy resolution with injection rather then requires, perhaps that's the best way to elegantly solve this once and for all.

E.g. Something like this: https://github.com/phillmac/orbit-db-managers/blob/develop/src/getManagers.js

[dp-kb]

Yes, this is a thorny issue! But even if I add a require shim and/or you use some other injection pattern, fs still won't exist in the browser so it'll break again. The central problem here is that isElectron is resolving to true when window.process is set. But window.process is a semi-common browser shim so isElectron returns a false positive when I've set that. So the issue is not with require.

I think the solution is to make a more accurate assessment of whether the app is an electron app. Then you don't have to change your require patterns at all (yet). Some discussion: https://github.com/electron/electron/issues/2288

The consensus is around:

var userAgent = navigator.userAgent.toLowerCase();
if (userAgent.indexOf(' electron/') > -1) {
   // Electron-specific code
}

OR

process.versions['electron']

OR using the following npm module. This is where you grabbed your code above from, though it's been updated in the meantime:

https://github.com/cheton/is-electron/blob/master/index.js

That last package is probably optimal if you're willing to add another dependency.

[tabcat]

hi @dp-kb , is this still an issue?

[cakecatz]

I also ran into this problem recently when I changed the build tool for my application.

I want to change it to use cheton/is-electron, is it ok to send a PR?

[tabcat]

@cakecatz yes please :)