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

Feature/add support for server and hybrid #1652

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

tmclane
Copy link
Member

@tmclane tmclane commented Jul 25, 2022

This is a first pass implementation of a Wails target type of server and hybrid.

wails build -outputType hybrid
wails build -outputType server

Both server and hybrid accept an optional options.Server argument to define the host and port on which to listen.
The intent was to allow the hybrid to enable / disable the server endpoint at runtime but this is not implemented at this time.

Server

  • only enables the dev server on the specified host and port (does not open a local window)

Hybrid

  • Always opens a local desktop window
  • Enables a remote access option using the devserver functionality on the specified host/port.

Default port is : 3112

@tmclane tmclane requested a review from leaanthony July 25, 2022 20:23
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jul 26, 2022

Deploying wails with  Cloudflare Pages  Cloudflare Pages

Latest commit: f4c9976
Status: ✅  Deploy successful!
Preview URL: https://40d74f09.wails.pages.dev
Branch Preview URL: https://feature-add-support-for-serv.wails.pages.dev

View logs

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 45a00f3 to 2b558ad Compare July 26, 2022 16:02
@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 2b558ad to f673d1a Compare August 5, 2022 21:12
@m29h
Copy link

m29h commented Aug 8, 2022

@tmclane I played a bit with your feature but I can't really get it to work.

My main issue is that an app compiled with -outputType server seems to never call the OnStartup and/or OnDomReady callbacks which takes away opportunity to store the context like I am used to along with all related functionality...

@tmclane
Copy link
Member Author

tmclane commented Aug 8, 2022

I'll look into that although it leads to a few questions.

  • Would you want the OnDomReady to be called each time a new client connects?
  • At what point is the OnStartup supposed to be called?

@tmclane
Copy link
Member Author

tmclane commented Aug 8, 2022

I don't expect the server to function exactly the same as the local version since it is multi-user capable by default. Note that each connection is it's own connection and does not share state automatically either.

@m29h
Copy link

m29h commented Aug 8, 2022

For my use case of 'server' I need the same functionality of wails such as RPC and events with the only difference that the app runs in the browser and there is no dependency of the app to a desktop window Operating system framework.

Think an wails app running on a raspberry pi and the user interface operated from any browser in the local network.

I would not have any requirements on shared state between two browser apps. For my use case 99% of time there would be only a single browser connected. I guess the cleanest solution would be if a new instance of every RPC backend app object would be created on any new browser connection. This would then also start the OnXXXX callback events each time for the new instance.

The probably cleaner way would be if every wails rpc call would pass a reference to the Caller context on each RPC call. However that seems to be beyond the current way wails works.

@tmclane
Copy link
Member Author

tmclane commented Aug 8, 2022

You've described my use-case as well. I'll look into getting the OnXXX events to occur.

@tmclane
Copy link
Member Author

tmclane commented Aug 10, 2022

Thank you for testing! I'll have an update soon hopefully time permitting.

@m29h
Copy link

m29h commented Aug 14, 2022

Travis,
I played around with your branch and committed my current status to m29/hfeature/add_support_for_server_and_hybrid

My main changes are enforcing the null frontend to call the OnStartup() callback and I also changed a bit the app_hybrid_server.go because the go.embed asset embedding was apparently broken.

There is a lot of duplicate/redundant code in the app_hybrid_server.go+app_dev.go+app_production.go files that could in principle be restructured given that all of them more-or-less do the same but with slightly different mechanisms.

Functionality wise I would be happy with the version on m29/hfeature/add_support_for_server_and_hybrid.

I even tested some edge-cases like browser/disconnect/reconnect and multiple paralell browser connections, and everything works within my use case.

Of course feel free to merge code from my branch into your PR.

@tmclane
Copy link
Member Author

tmclane commented Aug 14, 2022

I completely agree there is a ton of redundancy in the code. I was trying to go step by step instead of a huge rewrite as the first pass at this.
I'll be happy to incorporate your change into my branch for now and then will continue to improve it over time.

I'll kee rebasing until we get it merged to main.

@tmclane
Copy link
Member Author

tmclane commented Aug 14, 2022

I do feel the null front end will not be needed in the final version. It was just a workaround to be able to use the existing dev server code.

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from b949618 to d0a867b Compare August 15, 2022 02:48
@tmclane
Copy link
Member Author

tmclane commented Aug 15, 2022

@m29h Your commit has been included in this branch.

@tmclane tmclane marked this pull request as draft August 15, 2022 02:50
@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from d0a867b to bcc395b Compare August 19, 2022 02:29
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 21, 2022
@leaanthony leaanthony added TODO The issue is ready to be developed and removed wontfix labels Sep 21, 2022
@laoshaw
Copy link

laoshaw commented Oct 7, 2022

does this merge allow me to reuse the same (desktop) UI code on a http server? or the 'server' output is just here to help debugging?

@tmclane
Copy link
Member Author

tmclane commented Dec 5, 2022

@laoshaw the idea is that the same application would be able to be accessible over the network in a remote client session. Note that it does not share any state with a local window on the desktop. Multiple webclients can connect at the same time as well but are not linked together and are basically independent.
I am going to pick this up again in earnest real soon now.

@tmclane
Copy link
Member Author

tmclane commented Dec 5, 2022

It will be rebased and then updated to match the current state of the codebase.

@tmclane
Copy link
Member Author

tmclane commented Dec 5, 2022

Actually I was redoing this work in a new way. This may be discarded..

@alienator88
Copy link

This would be amazing for what I'm looking for. We have a cli tool at my work that has to be executed and authenticated locally by each user. Being able to run CLI commands in this tool locally and displaying in a browser window would be awesome. We'd like to get away from using a desktop GUI if possible and just have them launch the app and browse on localhost:port. Thanks for this work @tmclane!

@tmclane
Copy link
Member Author

tmclane commented Dec 5, 2022

I wasn't planning on adding authentication to the endpoint immediately. Sounds like that needs to be put on the roadmap as well.

@alienator88
Copy link

Oh sorry, I might've explained wrong. No auth needed from the wails side for my use case at least. Just the CLI tool we use internally that I mentioned in the example :)

@radoslaw-szpiech
Copy link

radoslaw-szpiech commented Mar 15, 2023

Is there any progress in this PR? It could be really useful in one of our projects at VEGA. We are experiencing some issues when testing our wallet app, which we believe may be caused by desktop app being run in the same time we're performing tests in browser.

@leaanthony
Copy link
Member

No progress. If you find a solution that works for you, it would be great to hear what you did.

@suntong
Copy link

suntong commented Aug 7, 2023

I have no issue with using Caddy in the future.

Agree -- if we cannot go there in one step, then make that a send step maybe is a good idea -- Releasing the feature first now, it will draw more people / talent over here and things will surely get easier, especially Caddy author himself had offered help.

The code draft I propose a week ago also would not introduce any significant amount of potentially "stale" code that is exclusively for the server use-case as it shares this server code with the current devserver. The idea is that it is more of a generalization of mechanisms that are already there today.

This is a really reasonable approach, and mature enough to be merged IMHO, as "This branch is 300 commits ahead". The longer to wait and the more difficult to merge maybe.

So please, wrap up what we have now so that our application can be either be a local desktop GUI program and a cloud-based solution now.

Please consider. thanks

@tmclane
Copy link
Member Author

tmclane commented Aug 7, 2023

300 commits ahead is merely a rebase away to being mostly gone :).

@suntong
Copy link

suntong commented Aug 7, 2023

Oh... ☺️☺️☺️

@tmclane
Copy link
Member Author

tmclane commented Aug 7, 2023

I may have more time to resume my work on this soon. Life has been hectic to say the least for me this summer.

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from bcc395b to ea84b93 Compare August 7, 2023 22:11
@mholt
Copy link
Contributor

mholt commented Aug 7, 2023

This probably isn't helpful as it could be incredibly niche, but just in case it spurs any ideas...

I have recently implemented an HTTP server for my Wails app in such a way that its core application logic (the app bindings, basically -- the functions that can be called by the frontend) can be accessed these ways:

  • JS function calls using Wails runtime/bindings
  • HTTP JSON API
  • CLI

Basically, I still have an App struct. And if there's a function with this signature:

func (a *App) DoSomething(a string, b int) (Result, error) {
   ...
}

then it can still be called by the JS frontend via Wails bindings like normal:

import { DoSomething } from "../wailsjs/go/main/App.js";

const result = await DoSomething("foo", 3.14);

Now, my app also has a Server struct inside it. The server is configured with a listener, maybe with some authentication, whatever you want. That's up to you. The key is its Handler, an *http.ServeMux. When I initialize the app, I add a bunch of routes to its mux, one per app function:

a.server.mux = http.NewServeMux()

// SPA static file server
a.server.staticFiles = http.FileServer(http.FS(a.frontend))
a.server.mux.Handle("/", Endpoint{
	Method:  http.MethodGet,
	Handler: a.server.serveFrontend,
})

// API endpoints
for command, endpoint := range a.commands {
	a.server.mux.Handle("/api/"+command, endpoint)
}

What is a.commands? It's simply a mapping of function names (aka. API endpoints) to info about that endpoint:

commands map[string]Endpoint

The key of this map is the last component of the API URI, e.g.do-something for a full URI of /api/do-something, and Endpoint is:

type Endpoint struct {
	Method      string
	ContentType string
	Payload     any // example of the input
	Handler     http.Handler
	Help        string // help text for CLI
}

For example:

a.commands = map[string]Endpoint{
	"do-something": {
		Handler:     a.server.handleDoSomething,
		ContentType: "application/json",
		Method:      http.MethodPost,
		Payload:     MyStruct{},
		Help:        "Does something!",
	},
...

The Endpoint type has a ServeHTTP() method so it is itself a handler, which decodes the payload for convenience of the actual handler about to be called (handleDoSomething in the example above):

func (e Endpoint) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	switch e.ContentType {
	case JSON:
		payload := reflect.New(reflect.TypeOf(e.Payload)).Interface()
		if r.ContentLength > 0 {
			err := json.NewDecoder(r.Body).Decode(&payload)
			// (handle err)
		}
		r = r.WithContext(context.WithValue(r.Context(), "payload", payload))
	}

	e.Handler(w, r)
}

Then handleDoSomething is this:

func (s *server) handleDoSomething(w http.ResponseWriter, r *http.Request) {
	payload := r.Context().Value("payload").(*MyStruct)
	result, err := s.app.DoSomething(payload.Foo, payload.Bar)
	jsonResponse(w, result, err)
}

(The jsonResponse() function simply handles the error if there is one, or serializes the result as JSON otherwise.)

With a little more code, this gives us a CLI for free now too, so I can do something like this:

$ myapp do-something --foo "foo" --bar 3.14

This is possible because we know how to look up "do-something" in the map of commands, and with the Endpoint struct filled out we know how to serialize the args into a JSON structure and make the HTTP request! (It can be a virtual request to be totally in-process. I actually call the ServeHTTP() method directly without using sockets.) This way, the HTTP API is also the gateway for the CLI. So yes there's some code to parse the CLI args and turn them into an HTTP request, and I can share that if desired.

My solution may be incredibly too niche, I realize. I just wanted to put it out here in case it inspires some ideas for the future.

Pros to this approach:

  • Business logic stays close/attached to the App itself, no duplicating!
  • HTTP, CLI, and GUI/JS access to business logic
  • The HTTP, CLI, and GUI/JS APIs are congruent/symmetrical, i.e. they look the same in all forms.
  • Adding or changing an endpoint involves copying+pasting just a little bit of boilerplate (just a few lines) and then tweaking the function call or the struct name, etc. Most of this could probably be automated!
  • CLI help text is generated automatically. (I actually haven't written this part yet.)

Cons to this approach:

  • Endpoints need code in a few places so they can be reached via HTTP and CLI in addition to just directly calling the function. (The small HTTP handler wrapper to convert from JSON struct to a function call, for instance.)
  • I'm not sure if the map of Endpoints can be automated, at least not entirely. Like help text would have to be typed in.
  • Not sure how easily "default" values could be defined for payloads; i.e. if a deserialized struct is empty, how to apply defaults and document them at the same time? Maybe could be done with a constructor function of sorts.

For me, the cons are pretty minor, and the benefits are big: I get 3 powerful ways to call my business logic, that all look and act the same, because under the hood they are the same. It's just a matter of linking the App function to an HTTP handler and a CLI.

Feel free to ignore this though, I realize it might seem complicated 🙃


For context: I actually had to stop using Wails for my app because my UI has pretty intensive stuff going on that Webkit2GTK chokes on: canvas, webgl, lots of media, videos, etc -- Wails itself is great but webkit2gtk keeps crashing and having numerous other issues, video driver problems, etc. So anyway, that's my motivation for working on this independently; I don't strictly need a hybrid model per-se, but my solution is close to one.

@suntong
Copy link

suntong commented Aug 8, 2023

Bravo @mholt!

HTTP, CLI, and GUI/JS access to business logic and stay close/attached to the App itself, and are also congruent/symmetrical.

This is exactly what I'll be needing, and I'm sure a lot of people would find it handy too. Thanks a lot for sharing, which I'm fully confident that it'll work with Caddy in the future if we need it, :).

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from ea84b93 to 658dbaf Compare September 27, 2023 15:01
@JensvandeWiel
Copy link

Looks great, can't wait for this to be merged!

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 658dbaf to ff3bcc6 Compare November 6, 2023 16:18
@FrenchGithubUser
Copy link

Yes this will breat a great addition !

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from ff3bcc6 to 783957f Compare January 4, 2024 14:19
Copy link
Contributor

coderabbitai bot commented Jan 4, 2024

Important

Auto Review Skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@flarco
Copy link

flarco commented Mar 2, 2024

hi @leaanthony any eta on this?

@tmclane
Copy link
Member Author

tmclane commented Mar 2, 2024

This isn't planned for inclusion at this time in v2.

This is left in the repo so that industrious users can leverage it and expand on it but doesn't really have official support. That being said I can rebase it sometime and push it up again and then you could update your reference to use it.

@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 783957f to 207b849 Compare March 11, 2024 16:14
@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 207b849 to 1a3657e Compare April 12, 2024 14:05
tmclane and others added 8 commits May 1, 2024 14:08
null.Frontend does nothing and is merely needed to pass to the devserver so
it has a frontend.Frontend to drive
- websocket client connection/disconnections
- server endpoint
go.embed asset handling in  production build
call the OnStartup() callback when running the null frontend
structure the app_hybrid_server.go to follow the structure and code of app_production.go
@tmclane tmclane force-pushed the feature/add_support_for_server_and_hybrid branch from 1a3657e to f4c9976 Compare May 1, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO The issue is ready to be developed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet