js-ipfs-utils icon indicating copy to clipboard operation
js-ipfs-utils copied to clipboard

Defer to native-fetch whenever `ELECTRON_RUN_AS_NODE` is set

Open rj-calvin opened this issue 4 years ago • 7 comments

You guys are using electron-fetch whenever the is-electron package detects if it is running in an electron process (particularly, the main process). However, is-electron seems to be returning true even when in a process that has been forked from electron's main process as a node server.

This is currently what I am trying to set up in my own application, however, since you can't access electron from within a forked process, attempting to load electron-fetch results in an error wherein electron claims that it hasn't been installed properly.

The environment variable ELECTRON_RUN_AS_NODE is used to observe when a process is running as a fork from main, so I was thinking an additional condition could be added to defer to native-fetch whenever it is set.

rj-calvin avatar Aug 12 '21 15:08 rj-calvin

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review. In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment. Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

  • "Priority" labels will show how urgent this is for the team.
  • "Status" labels will show if this is ready to be worked on, blocked, or in progress.
  • "Need" labels will indicate if additional input or analysis is required.

Finally, remember to use https://discuss.ipfs.io if you just need general support.

welcome[bot] avatar Aug 12 '21 15:08 welcome[bot]

This sounds like it should be a PR to is-electron - have you opened an issue there?

achingbrain avatar Aug 12 '21 16:08 achingbrain

I considered that, but I believe that it's likely that this would be a breaking change there.

rj-calvin avatar Aug 12 '21 17:08 rj-calvin

Blocked for now: waiting for Node 16 to become LTS, and maybe even Electron to ship ELECTRON_RUN_AS_NODE with implementation matching Node 16+ changes around node-fetch.

lidel avatar Oct 01 '21 14:10 lidel

Blocked for now: waiting for Node 16 to become LTS, and maybe even Electron to ship ELECTRON_RUN_AS_NODE with implementation matching Node 16+ changes around node-fetch.

If anyone needs to have this issue fixed in the meantime, I have a branch with a workaround here: https://github.com/gatsby-tv/js-ipfs-utils

One would only need to add the following to their package.json:

{
  "resolutions": {
    "ipfs-utils": "https://github.com/gatsby-tv/js-ipfs-utils#fix/defer-to-native-fetch"
  }
}

rj-calvin avatar Oct 03 '21 00:10 rj-calvin

Same issue here. This is also broken in workers.

The electron.net API should only be used when process.type === "browser". Technically it's still an electron process (as for example process.versions.electron etc. is available) but it's not the main (browser) process. IMHO is-electron is right as it only tells its "some" electron process.

Changing https://github.com/ipfs/js-ipfs-utils/blob/5eecdcbd81f5448ba1d4c0d2ad2050d0ceb240a0/src/env.js#L8 to

const IS_ELECTRON_MAIN = IS_ELECTRON && !IS_ENV_WITH_DOM && typeof process !== 'undefined' && process.type === "browser"

should do the trick.

dkuhnert avatar Sep 29 '22 14:09 dkuhnert

@dkuhnert I don't see why the ELECTRON_RUN_AS_NODE variable wouldn't be sufficient and simpler.

rj-calvin avatar Oct 03 '22 17:10 rj-calvin