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

feat: add new APIs and example #58

Merged
merged 8 commits into from
Jul 12, 2024
Merged

Conversation

sensiblearts
Copy link
Contributor

This PR surfaces webui.get_free_port(), from the latest build.

And to test it, a new example called, custom_spa_server_on_free_port.

Why this example?

I'm doing something like this with SvelteKit, so I built an example to test the approach. Use it or not, or alter it as you see fit.

Question: Is there something about my example that is causing click and navigation events to be dropped?

(This example does not need those events -- only EVENT_CONNECTED and EVENT_DISCONNECTED, which are working fine -- but I need to figure out whether it's my code or an API change or a break.)

Thanks.

@jinzhongjia
Copy link
Collaborator

Okay, good PR, but there are some problems with the code. Can you let me modify it directly on your PR?

@sensiblearts
Copy link
Contributor Author

p.s., I'll have to update this after your comments; does that mean that I should have created this as a "draft" PR?

@jinzhongjia
Copy link
Collaborator

I will update all the latest newly added APIs soon
No need to convert to draft

@sensiblearts
Copy link
Contributor Author

Your comment appeared after I submitted mine. Lol.

Sure. How do I give you permission?

@jinzhongjia
Copy link
Collaborator

Haha, actually you gave me permission

@jinzhongjia
Copy link
Collaborator

pic

@sensiblearts
Copy link
Contributor Author

Any time I submit a PR, just tell me what changes you want and I'll take care of it.

I can do more after work tomorrow.

Thanks.

@sensiblearts
Copy link
Contributor Author

Also, if it's easy to a build step to copy all example .exe and supporting files (e.g., html) into their own subfolder -- it would be a good pattern across all webui platform builds, I think.

@jinzhongjia
Copy link
Collaborator

I think I should have fixed it, just use zig build run_custom_spa_server_on_free_port, it will work

But I'm not sure whether the running effect is what you need. You still need to confirm it.

@jinzhongjia jinzhongjia marked this pull request as draft July 11, 2024 02:06
@sensiblearts
Copy link
Contributor Author

Thank you. I guess I didn't need to write all that copy files code! I'm learning.

I'll keep future PRs clean of that kind of stuff, or mark them as drafts.

@jinzhongjia jinzhongjia changed the title surface WebUI.webui_get_free_port(); + an Example feat: add new APIs and example Jul 11, 2024
@jinzhongjia
Copy link
Collaborator

Add APIs:

  • webui_get_free_port
  • getPort
  • getMimeType

@sensiblearts
Copy link
Contributor Author

I have to get to sleep; I'll check back tomorrow.

@sensiblearts
Copy link
Contributor Author

@jinzhongjia would you like me to work on the spa/free_port example above? Just let me know if you want me to do anything. I don't want to spend time on it if you're doing it. Thanks.

@jinzhongjia
Copy link
Collaborator

so, you think this pr now is ok?
Then we can merge it

@sensiblearts
Copy link
Contributor Author

Do you mean the whole PR (surfacing getFreePort, the stuff you added, my example app)? Or just the new API stuff.

Can you selectively merge, or is it all-or-nothing?

I'm not familiar with git workflow when 2 people have worked on a single pull request.

Before it's merged, I need to remove that copy file junk that I put in build.zig; how can I do that without overwriting what you've done?

Sorry I don't know more git. I've worked alone most of the time.

@jinzhongjia
Copy link
Collaborator

When two people handle a pull request, it can be regarded as two people using a branch together. You only need to pull regularly. You only need to delete and then push.

@sensiblearts
Copy link
Contributor Author

Ok. I'll pull to get your edits. Then I'll clean up my stuff. Then push.

@sensiblearts
Copy link
Contributor Author

I removed my commented-out code in build.zig. So you can merge if you feel it's ready.

However, there may still be a problem that will show up again. It's not a problem with this PR, I don't think.

The problem:

Yesterday, @hassandraga fixed the missing events not being delivered to the handler (click, navigation) , which caused navigation to trigger a disconnect, which he then fixed. I pulled his changes, and at first it did not seemed fixed; so I cleaned zig-cache and zig-out and tried again, and still not fixed. Then I came back an hour later and ran it and it was fixed. And my own app (a SvelteKit SPA) is working fine like that.

Now, navigation seems to be again causing disconnect. E.g., in custom_web_server example, navigation to second.html disconnects and does not reconnect (to second.html's webui.js).

Anyway, try to run the custom_web_server example and see what you think.

Problems that come and go, in a multi-threaded system, make me worry. But I could not be paying attention closely enough to what I'm doing.

@hassandraga
Copy link
Member

Problems that come and go, in a multi-threaded system

I did not see that issue in C. Probably it's the wrapper, or how the wrapper is interacting with the C library.
However, let's figure out.

@sensiblearts
Copy link
Contributor Author

I can't work on it tomorrow, but Saturday I'll try to debug it. First, I'll see if it happens with other wrappers. Then I'll build webui in debug mode and see what I can figure out. I'd like to gain a deeper understanding of webui's internals, anyway.

@hassandraga
Copy link
Member

Sounds good @sensiblearts. You can also try to disable the multi-threading to see if this improve the UX.
webui_set_event_blocking(win, true);

@jinzhongjia
Copy link
Collaborator

I update zig-webui to the latest webui
is this ok?
image

@sensiblearts
Copy link
Contributor Author

@jinzhongjia it looks fixed. I think you can merge and then I'll pull and see if the problem is gone on my machine.

@jinzhongjia jinzhongjia marked this pull request as ready for review July 12, 2024 10:15
@jinzhongjia jinzhongjia merged commit 3a7e8b4 into webui-dev:main Jul 12, 2024
13 checks passed
@sensiblearts
Copy link
Contributor Author

sensiblearts commented Jul 12, 2024

@jinzhongjia @hassandraga I just pulled this newly merged PR, just after the c_allocator commit. Results:

  • on WSL Ubuntu 20 does NOT cause Disconnect, works fine.
  • on Windows 10, still causes Disconnect and crash on Navigate link. My machine:

image

I'll dig into it tomorrow.

@jinzhongjia
Copy link
Collaborator

aha,I do not have machine with windows,maybe I can not help more 😞

@hassandraga
Copy link
Member

Thank you @sensiblearts for the report. I'm working on it.
Just to let you know, I was focusing on the C examples only, now after all needed APIs are implemented, it's time to fix wrappers issues and needs, then release a stable version.

@hassandraga
Copy link
Member

std.debug.print("URL: {s}\n", .{url});
std.debug.print("LEN: {d}\n", .{len});

// we generate the new url!
const new_url = allocator.allocSentinel(u8, len, 0) catch unreachable;

Logs:

[Core]          _webui_get_cb_index([1])
[Core]          _webui_get_cb_index() -> Element: []
[Core]          _webui_get_cb_index() -> Found at 1
[Core]          _webui_window_event() -> Calling all-events user callback
[Call]
[User] webui_get_string()
[User] webui_get_string_at([0])

URL: http://localhost:8080/index.html
LEN: 32
Segmentation fault at address 0xffffffffffffffff

As far I can tell, WebUI gives the correct pointer all the time, but copying the url in Zig causes the crashe, not C.

However, the wrapper should convert the C const char* to [:0]const u8, or whatever Zig developers usually use. Or making the Zig setters APIs accept whatever Zig getters APIs gives. That what I did in the Python wrapper.

@hassandraga hassandraga added the bug Something isn't working label Jul 12, 2024
@jinzhongjia
Copy link
Collaborator

em...
I will try fix it tomorrow

@sensiblearts
Copy link
Contributor Author

sensiblearts commented Jul 12, 2024

This causes disconnect and crash, too, suggesting that it's the navigate call:

 switch (e.event_type) {
        .EVENT_CONNECTED => {
            std.debug.print("Connected. \n", .{});
        },
        .EVENT_DISCONNECTED => {
            std.debug.print("Disconnected. \n", .{});
        },
        .EVENT_MOUSE_CLICK => {
            std.debug.print("Click. \n", .{});
        },
        .EVENT_NAVIGATION => {
            const win = e.getWindow();
            win.navigate("http://localhost:8080/second.html");
        },
        else => {},
    }

@hassandraga forgive my memory, but after I run mingw32-make debug to generate the webui debug DLL, what else do I have to do to get zig-webui project to link it? I got it working once before but I forget what I did. I feel like I copied it to another folder...?

UPDATE: It does the same thing in the webui custom_web_server example -- no Zig involved.

@hassandraga
Copy link
Member

If we are talking about the C then just run mingw32-make debug in webui folder, then run the same command in the examples folder.

If we are talking about Zig, I really don't know how to properly doing it. I don't maintain the Zig wrapper. @jinzhongjia is the best person to ask.

@sensiblearts
Copy link
Contributor Author

(Interesting: I've not spent much time on the C examples, but when I did, I've been running the command zig build examples and then copying the resulting .exe into the example folder. E.g., zig build examples results in a custom_web_server.exe that I can copy into that example's folder and run. I won't do that anymore!)

Anyway, here are the C make results for custom_web_server example on Windows 10:

mingw32-make debug:

  • main.exe runs fine: when you navigate, it disconnects but then instantly reconnects to the other page's webui.js
  • Also, I scanned the debug logs and saw no hint of any error or crash
  • Inspecting the browser console shows what you would expect, 200

mingw32-make:

  • When main.exe is run, it looks ok until I click the nav link. When I do that
    • browser console: WebUI -> CMD -> Navigation [http://localhost:8080/second.html] webui.js: 2
    • nothing happens -- it does not go to second.html
    • (I suspect that it has disconnected and wait() has returned.)
  • So I click the same link (on page index.html) a second time, and it does go to second.html, but it is not connected:

second html
And I click to navigate back to index.html and I get a similar result:
index html

@hassandraga
Copy link
Member

Thank you for the report @sensiblearts. to be honest, I completely forgot to test the release mode, I only test the debug one. Let me check this.

@sensiblearts
Copy link
Contributor Author

Ok,thanks.

Is there a way to printf in release mode?

What if webui had a C console.log method, that made a JS call?

hassandraga added a commit to webui-dev/webui that referenced this pull request Jul 13, 2024
@jinzhongjia
Copy link
Collaborator

the new commit of webui seems fix this problem

@sensiblearts
Copy link
Contributor Author

@jinzhongjia do you mean it's fixed on Windows?

@jinzhongjia
Copy link
Collaborator

jinzhongjia commented Jul 13, 2024

@jinzhongjia do you mean it's fixed on Windows?

I didn't test with windows, but I found that custom_web_server will also experience disconnection behavior under linux. But now it works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants