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: improve ui proxy config #213

Open
0x70b1a5 opened this issue Aug 5, 2024 · 5 comments
Open

feature: improve ui proxy config #213

0x70b1a5 opened this issue Aug 5, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@0x70b1a5
Copy link

0x70b1a5 commented Aug 5, 2024

Is your feature request related to a problem? Please describe.
I'm always frustrated when creating a new kinode app with a UI, which needs to make requests to public assets, for example, calendar.png, at the root URL of the web UI.

As is, the universal anti-regex here triggers a proxying of the request from the browser for /calendar.png (or any other route not specified by the regex, whether it's for an asset or not) to the node, which throws out a 404.

The current solutions are either to manually change the regex to match all of the possible assets I could fetch, or change the config around from an "implicit proxy, with exceptions" to "explicit proxy, based on prefix" method.

Describe the solution you'd like
Rather than implicitly proxying everything that does NOT match an eldritch regex to the node, I'd suggest that the UI decide on some explicit prefix to proxy to the node. For example, /api/ or /node/.

Describe alternatives you've considered
I can of course just fix all of my own apps in this way, but it's probably better to do this universally to avoid headaches for others

@0x70b1a5 0x70b1a5 added the enhancement New feature or request label Aug 5, 2024
@0x70b1a5
Copy link
Author

0x70b1a5 commented Aug 5, 2024

I can submit a PR for this at some point but may be unable to give it the thorough treatment it deserves regarding review and edge case testing

@dr-frmr
Copy link
Member

dr-frmr commented Aug 8, 2024

I think this is actually an issue for the kit repo (@nick1udwig what do you think?). The runtime doesn't set defaults here, the UI templates in kit do.

@nick1udwig
Copy link
Collaborator

Yeah, does seem like kit.

@0x70b1a5 can you provide me with some context as to why this "implicit proxy" exists in the first place? What sort of requests are getting proxied?

@0x70b1a5
Copy link
Author

You may be familiar with how React apps use fake URL paths to guide page "navigation" between different components. You can see in the regex there a few routes matching the built files generated by Vite, which would be the files we'd want to get from the node's pkg/ui folder over HTTP when built.

So usually in a single page app, we'd want to proxy all unknown routes (/images/blah.png, for example) to the the server, because we just want to GET /blah.png and show it. In this case the node is the server, and in addition to images, it's stuff like /our.

IIUC the issue is that the built code and the dev server code need to behave differently. The node just makes requests to itself, and since it has the built files served over HTTP everything's good. But the dev server isn't actually making requests from the node - it's usually on localhost:5173. So we need to somehow tell the Vite dev server to NOT proxy low-level framework calls, but rather preserve them as calls to :5173, WHILE proxying everything else like /images/cat.png, to the node. Maybe something is here but I didn't get around to it in time (apologies). Anyway, that's why all of the dev-server-specific requests are specifically called out in that regex; because otherwise they go to the node which doesn't respond (or worse, the node DOES respond with an outdated version of the code from the last time you built and installed!)

This came up when Zahir was making an app. It worked fine as a built-and-installed ui package, but when he tried to work on the dev server, none of the initial requests were getting him the hot-reloaded supertypescript whatevers. I think I debugged this with Drew as well before he left. I'm pretty sure @willbach wrote this code and so if I have made errors in diagnosing the problem and/or solution, he would be able to tell you.

@dr-frmr dr-frmr transferred this issue from kinode-dao/kinode Aug 15, 2024
@0x70b1a5
Copy link
Author

This might also be a "skill issue" where the ecosystem of app dev requires we not use these Vite features. Working on this more myself recently, a fix to get consistent behavior was simply to not use the proxy at all, and just explicitly hardcode the full process path into every HTTP call (especially when calling out to different processes than your main one). But, if that's ideal, then the implicit proxy rule should also be removed so that it doesn't interfere with asset GETs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants