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.

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

Fixes to getting Electron to work #373

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kakaroto
Copy link

@kakaroto kakaroto commented Aug 4, 2020

This PR includes 3 fixes. The first one is for #371 which I ended up forking/fixing, the other two were done in an attempt to get my app to run in electron for those that do not have chrome available. I wanted to include electron in my app, and found all sorts of issues, which this finally fixes.

The first issue I found is that setting the mode to "electron" on windows will cause an exception because if which returns None, you try a os.path.join(None, "../node_modules/etc...") which will fail.

The second issue is that I realized that no matter what I did, it wouldn't work because the run command adds "." as an argument, and while removing it fixed it, I believe you had that in there on purpose because it specifies the current working directory for an electron bundle, and I didn't want to change that as it would break for those using a different system than what I'm doing.

Basically, I just downloaded the electron binaries from their releases, extracted it under a subdir, added it to the PATH, and I want to run it directly from there. There is no bat file, and no node_modules here, and launching electron with "." as its first argument will fail because my app's current dir is not an electron bundle. Using "custom" mode was the only solution, but that launches cmdline_args but without adding the url to it, and since I use port=0 I had no way of hardcoding the url in the cmdline_args option either (and doing a block=false, and then trying to get the port to launch manually, and losing that whole websocket auto close thing, etc.. felt way too complicated than just adding a callback for custom modes).

So that's the explanation around where these commits are coming from, hopefully this gets accepted.
Thanks!

cx_freeze frozen apps don't define/use _MEIPASS
Fixes python-eel#371
If electron isn't found on windows, the os.path.join will
complain about passing NoneType to it
Also, return the original electron found if if the path that was found
doesn't have a relative ../node_modules/etc.. file
This allows bundling of electron.exe directly in an app
This allows us to specify the custom command to launch and passing
any argument to it, including the URL of the server that was just created.
Without this option, there is no way to run a custom command
while also giving it the actual url to launch. Hardcoding the url in
cmdline_args being unfeasable if using port=0 for example.
Copy link

@bitnom bitnom left a comment

Choose a reason for hiding this comment

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

Shouldn't it be .\node_modules\... since .. is 1 directory up/not the programs working dir? I was just trying to get electron/eel working on Win so I installed your master via pip. When I ran the electron example after doing npm i, I got:

Traceback (most recent call last):
  File ".\hello.py", line 19, in <module>
    eel.start('hello.html', mode='electron')
  File "C:\Python38\lib\site-packages\eel\__init__.py", line 145, in start
    show(*start_urls)
  File "C:\Python38\lib\site-packages\eel\__init__.py", line 173, in show
    brw.open(start_urls, _start_args)
  File "C:\Python38\lib\site-packages\eel\browsers.py", line 68, in open
    raise EnvironmentError("Can't find %s installation" % browser_module.name)
OSError: Can't find Electron installation

so is your code assuming the programmer will use something like electron-forge to get a single .exe? I figured I would just do npm i and then bundle it all up with pyinstaller. In that case it should be looking in .\node_modules\electron\dist\electron.exe afaik.

@kakaroto
Copy link
Author

Shouldn't it be .\node_modules\... since .. is 1 directory up/not the programs working dir? I was just trying to get electron/eel working on Win so I installed your master via pip.

I'm not exactly sure, I think the use of the node_modules path is somehow linked to the specific use case of the author here. I think that maybe the which is supposed to find electron.exe in a bin directory or something like that.

The way I did it is that I didn't use npm at all, I just downloaded the electron executable from https://github.com/electron/electron/releases and extract it in an Electron folder, then I use this :

  eel.start('index.html', port=0, mode="custom", custom_callback=self.PopenElectron)

with the PopenElectron function defined this way :

    def PopenElectron(self, args, urls):
        if platform.system() == 'Darwin':
            path = "Electron.app"
            if not os.path.exists(path):
                path = os.path.join(os.path.dirname(os.path.dirname(sys.executable)), "Resources", "Electron.app")
            cmd = ["open", "-a", path, "--args"]
        else:
            cmd = ["electron/electron"]
        cmd += args + [';'.join(urls)]
        return subprocess.Popen(cmd)

Which allows me to execute it it from my app's electron directory on windows, or from Electron.app on Mac, or from the Resources/Electron.app folder in my cx_freeze frozen app for Mac.

@bitnom
Copy link

bitnom commented Aug 19, 2020

I'm not exactly sure, I think the use of the node_modules path is somehow linked to the specific use case of the author here. I think that maybe the which is supposed to find electron.exe in a bin directory or something like that.

Let's do this then (Works for me):

def find_path():
    if sys.platform in ['win32', 'win64']:
        # It doesn't work well passing the .bat file to Popen, so we get the actual .exe
        bat_path = wch.which('electron')
        if bat_path is not None:
            exe_path = os.path.join(bat_path, r'..\node_modules\electron\dist\electron.exe')
            if os.path.exists(exe_path):
                return exe_path
            else:
                return bat_path
        else:
            exe_path = r'.\node_modules\electron\dist\electron.exe'
            if os.path.exists(exe_path):
                return exe_path
        return None
    elif sys.platform in ['darwin', 'linux']:
        # This should work find...
        return wch.which('electron')
    else:
        return None

@kakaroto
Copy link
Author

Yeah, that makes sense, that would work and is less confusing to me with regards to how which is supposed to find a .bat file in a directory that isn't within the node_modules dir.

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.

2 participants