python-eel / Eel

A little Python library for making simple Electron-like HTML/JS GUI apps
MIT License
6.44k stars 587 forks source link

Add eel routes to `Bottle` instances passed in to `eel.start` #573

Closed Sentinent closed 1 year ago

Sentinent commented 2 years ago

Addresses https://github.com/ChrisKnott/Eel/issues/572. Regression of https://github.com/ChrisKnott/Eel/issues/211 caused by https://github.com/ChrisKnott/Eel/pull/229.

Currently, eel routes are added via bottle.route(), which only adds routes to the default app. This means passing any non default instance of Bottle to eel.start() wouldn't work.

app = bottle.Bottle()
# ...
eel.init('...')
eel.start('...', app=app) # would not serve routes needed for eel to work

The behavior was changed to this in #229 to address #228 to allow for the following code to work:

app = beaker.middleware.SessionMiddleware(bottle.app(), ...) # bottle.app() == bottle.default_app()
eel.start('...', app=app)

However, the fix only applies if the middleware is constructed with the default app. The following would not work.

app = beaker.middleware.SessionMiddleware(bottle.Bottle(), ...)
eel.start('...', app=app)

To allow for configuration of middleware, a new function eel.add_eel_routes() is exposed. I believe this is a more robust solution as it allows for any Bottle to be configured properly, while still being wrapped in middleware, i.e.

app = bottle.Bottle()
eel.add_eel_routes(app) # only needed if we are wrapping `app` later
middleware = beaker.middleware.SessionMiddleware(app)
eel.start('...', app=middleware)

Backwards Compatibility

add_eel_routes() is automatically called on instances of Bottle passed to eel.start(), so no additional code is needed if not using middleware.

We default to the old behavior (adding eel routes to the default application) if eel.start() receives an object that is not a Bottle.

This should make it so that no existing code is affected by this change.


This PR also adds a test for the expected behavior.

samuelhwilliams commented 1 year ago

This looks good, thanks for putting up a PR and sorry for the slow reply @Sentinent.

Would you mind adding a line to the README under the app config option?

Aware our docs are not very good at the moment, but would still be nice to have something there about this :)

Edit: also if you could rebase on to latest master that would be fab.

samuelhwilliams commented 1 year ago

Thanks for the contribution 🙌