[assetserver] Inject runtime/IPC into all index html files #2203

stffabi · 2022-12-13T15:49:14Z

It will also be injected into all html files returned when requesting a folder path.

Fixes #2047

mholt · 2022-12-13T16:30:43Z

Cool, thank you for doing this 🥳 I will try to test it soon

v2/internal/frontend/assetserver/assetserver.go

leaanthony · 2022-12-14T12:03:07Z

I need to stop trying to review on the mobile app 😅

leaanthony

Thanks for doing this @stffabi !

v2/internal/frontend/assetserver/assethandler.go

mholt · 2022-12-20T19:46:48Z

@stffabi Is it expected that when my frontend does a fetch request for appbar.html it injects vite script into the response? (This seems to happen both with and without the patch, unless I messed up my test setup, which is quite likely, lol. I'm still only at about 50% brain capacity right now as I recover from sickness.)

In this case I'm fetching some HTML components that get injected into the page, and it already has vite; basically there are only certain files I want the script injected onto; and I'm starting to wonder if I should either just do that manually, or if I'm designing my app wrong...

mholt · 2022-12-21T06:31:00Z

One more question: Should the runtime and IPC be injected into the root index.html file even if the URL is not /? I'm trying an SPA for something, and currently I can only get the runtime and IPC scripts to be added if the URL is / and not /foo/, even though the response for /foo/ is also the contents of /index.html just like / is, except without runtime & IPC.

stffabi · 2022-12-21T06:38:55Z

One more question: Should the runtime and IPC be injected into the root index.html file even if the URL is not /? I'm trying an SPA for something, and currently I can only get the runtime and IPC scripts to be added if the URL is / and not /foo/, even though the response for /foo/ is also the contents of /index.html just like / is, except without runtime & IPC.

Yeah that's what this PR should do, it should inject into all html
Files, depending on the content type of the result when retrieving a folder url.
Can you please provide a minimal repro for this?

mholt · 2022-12-21T19:04:51Z

@stffabi Thanks. Yeah, I think so:

$ wails init -n myproject -t vanilla

I think this step is optional, but I already did it, so I'll document it here: add a file called html/foo/index.html in the frontend directory, with a simple hello world: <p>Hello world!</p>

Run wails dev, then curl http://localhost:34115/ | grep ipc.js and you will see that the IPC is injected. But run curl http://localhost:34115/foo/ | grep ipc.js and you will see that it is not, even though if you remove the grep it's the same /index.html file that's being served.

Shouldn't IPC+runtime be injected into the root index file regardless of the URL path?

Thanks for your help!

stffabi · 2022-12-21T20:08:44Z

@mholt thanks for this guide. I've just tested it with this PR and everything worked as expected.

Both curl curl http://localhost:34115/ | grep ipc.js and curl http://localhost:34115/foo/ | grep ipc.js returned successfully the patched HTML file and contained both ipc.js and runtime.js

I've tested on macOS but wouldn't expect this to differ between platforms. Would it be possible for you to check if you are using the right wails replace in your go.mod? Just to be sure.

replace github.com/wailsapp/wails/v2 v2.2.0 => github.com/stffabi/wails/v2 v2.0.0-beta.20.0.20221220124958-d3b156dff78e

mholt · 2022-12-21T20:19:30Z

Oh, I think I had the replace wrong 🤦 Thanks

Now I'm getting it injected twice: once on the root index.html, and then after my page fetches /foo/index.html and injects its response into <body>, it's included there too. And I'm getting errors like Callback 'application.App.SearchItems-3449944401' not registered!!! -- probably something dumb on my end, but I just wanted to double-check.

Thanks!

mholt · 2022-12-23T02:09:25Z

@stffabi I had a chance to go over this with @leaanthony which helped clear up my misunderstandings. And I am better equipped to clarify my request :)

So, even with this patch, if you do:

wails init -n myproject -t vanilla
wails dev
curl -v http://localhost:34115/foo

you'll notice that the HTTP response has the contents of /index.html, even though /foo was requested. This is the correct and expected behavior of Wails, serving index.html because there's no file called foo. However, if you grep ipc.js or grep runtime you won't find them. Yet if you curl -v http://localhost:34115/ then you will see both ipc.js and runtime.js are injected.

I think what needs to happen is Wails should be injecting runtime and IPC into the index file regardless of the URL path, or regardless of whether a file is found at the URL path, I mean. Lea pointed to this code:

	path := req.URL.Path
	switch path {
	case "", "/", "/index.html":
		recorder := httptest.NewRecorder()
		d.handler.ServeHTTP(recorder, req)
		for k, v := range recorder.HeaderMap {
			header[k] = v
		}

		switch recorder.Code {
		case http.StatusOK:
			content, err := d.processIndexHTML(recorder.Body.Bytes())
			if err != nil {
				d.serveError(rw, err, "Unable to processIndexHTML")
				return
			}
			d.writeBlob(rw, indexHTML, content)

		case http.StatusNotFound:
			// *** right here, might need to processIndexHTML? ***
			d.writeBlob(rw, indexHTML, defaultHTML)

This is needed, I think, because not all SPAs use hashes for routing. In other words, some use window.location.hash for routing, but fragment URLs aren't desirable or possible sometimes, so window.location.pathname is used for routing instead, which results in the Wails backend getting requests for file paths that don't exist (in which case it should still serve /index.html as a fallback, since that's the SPA root -- but with runtime & IPC!).

Does that make sense?

I'm sorry for the confusion about this issue, I hope this helps clarify what I need, at least, and what I think is correct behavior. :)

(PS. I also just barely discovered https://wails.io/docs/guides/frontend#overriding-default-script-injection which allows me to disable auto-inject and manually inject the scripts myself! That's very handy!)

leaanthony · 2022-12-23T03:42:38Z

Perhaps we could introduce a flag to indicate what to do when URLs are "not found"? I'm thinking that an "SPA" mode could be used as well as allowing the dev to supply an html file to serve in this instance. Thoughts?

mholt · 2022-12-23T05:27:48Z

@leaanthony Doesn't the AssetServer already invoke a user's handler if a resource isn't found?

leaanthony · 2022-12-23T06:10:06Z

I believe we present a default 404 page.

stffabi · 2022-12-23T09:30:50Z

@mholt I suspect there's still some confusion about all of this.

you'll notice that the HTTP response has the contents of /index.html, even though /foo was requested. This is the correct and expected behavior of Wails, serving index.html because there's no file called foo.

What you are seeing that this curl curl -v http://localhost:34115/foo returns index.html is a vite thing and is not a functionality of Wails itself and therefore only applies to wails dev. In a production build you would see just a 404 when fetching the /foo url.

The code you pointed to is before the request is handed over to vite, and vite does internally return the index.html when returning from d.handler. So wails code is not involved in all of this and the location // *** right here, might need to processIndexHTML? *** is never reached, because vite returns a 200 status. Doing a curl to vite directly curl -v http://localhost:3000/foo one can see that it already does this fallback thing and returns a 200 status.

I think what needs to happen is Wails should be injecting runtime and IPC into the index file regardless of the URL path, or regardless of whether a file is found at the URL path, I mean.

I can't see any way how we could detect that vite is returning the index.html for a missing /foo, unless there's a way to disable this behaviour in vite and doing this fallback handling in Wails itself.

Maybe we should meet us all together in a discord channel for a more 1:1 discussion on all of this. Maybe that would be easier for discussing this.

@leaanthony

Perhaps we could introduce a flag to indicate what to do when URLs are "not found"? I'm thinking that an "SPA" mode could be used as well as allowing the dev to supply an html file to serve in this instance. Thoughts?

This could be achieved by the developer with an AssetServer middleware and by adding custom RuntimeInjectionMatchers.

stffabi · 2022-12-23T09:31:47Z

@leaanthony Doesn't the AssetServer already invoke a user's handler if a resource isn't found?

If the file is not found on the Assets fs.FS it goes to the AssetHandler.

leaanthony · 2022-12-23T09:41:12Z

Agree about the discord chat. Seems like the async chat here is confusing us all :-) Like I was amazed vite returns data on :34115 :-)

leaanthony · 2023-02-23T06:27:43Z

@mholt - do you think this is good to go as is?

mholt · 2023-02-24T18:20:47Z

Sorry I stepped away from this for a while. Once I discovered <meta name="wails-options" content="noautoinject" /> I was using that. And at one point it was working, however now it seems to not be working. This is my HTML:

<!DOCTYPE html>
<html>
	<head>
		<title>Timelinize</title>

		<meta charset="utf-8">
		<meta name="viewport" content="width=device-width, initial-scale=1.0">

		<!-- add wails runtime and IPC scripts ourselves -->
		<meta name="wails-options" content="noautoinject" />
		<script src="/wails/ipc.js"></script>
		<script src="/wails/runtime.js"></script>
...

But it still seems to be injected by Wails (you can see the script tags twice; the ones at the top are injected):

This is confirmed by the console, which says that wails is connected... twice:

I might be misunderstanding something about this still...

It will also be injected into all html files returned when requesting a folder path.

stffabi · 2023-10-23T06:33:36Z

@leaanthony I've just rebased the PR to the master branch.

stffabi mentioned this pull request Dec 13, 2022

Runtime and IPC only injected on index.html file #2047

Closed

leaanthony reviewed Dec 13, 2022

View reviewed changes

v2/internal/frontend/assetserver/assetserver.go Outdated Show resolved Hide resolved

stffabi marked this pull request as ready for review December 14, 2022 12:34

stffabi force-pushed the feature/assetserver-all-index-inject branch from c89eda2 to d5dda99 Compare December 14, 2022 12:35

leaanthony approved these changes Dec 20, 2022

View reviewed changes

v2/internal/frontend/assetserver/assethandler.go Outdated Show resolved Hide resolved

v2/internal/frontend/assetserver/assethandler.go Outdated Show resolved Hide resolved

stffabi force-pushed the feature/assetserver-all-index-inject branch from b27da7d to d3b156d Compare December 20, 2022 12:49

stffabi mentioned this pull request Dec 23, 2022

[assetHandler] Add request path checks #2235

Merged

stffabi force-pushed the feature/assetserver-all-index-inject branch from e6493db to 5ad4ef5 Compare December 27, 2022 19:22

[assetserver] Inject runtime/IPC into all index html files

6442de3

It will also be injected into all html files returned when requesting a folder path.

stffabi force-pushed the feature/assetserver-all-index-inject branch from 5ad4ef5 to 6442de3 Compare October 23, 2023 06:37

leaanthony approved these changes Oct 23, 2023

View reviewed changes

leaanthony merged commit 30d17a7 into wailsapp:master Oct 23, 2023
4 of 8 checks passed

stffabi deleted the feature/assetserver-all-index-inject branch October 24, 2023 05:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[assetserver] Inject runtime/IPC into all index html files #2203

[assetserver] Inject runtime/IPC into all index html files #2203

stffabi commented Dec 13, 2022

mholt commented Dec 13, 2022

leaanthony commented Dec 14, 2022

leaanthony left a comment

mholt commented Dec 20, 2022 •

edited

mholt commented Dec 21, 2022

stffabi commented Dec 21, 2022

mholt commented Dec 21, 2022

stffabi commented Dec 21, 2022

mholt commented Dec 21, 2022

mholt commented Dec 23, 2022 •

edited

leaanthony commented Dec 23, 2022

mholt commented Dec 23, 2022

leaanthony commented Dec 23, 2022

stffabi commented Dec 23, 2022

stffabi commented Dec 23, 2022

leaanthony commented Dec 23, 2022

leaanthony commented Feb 23, 2023

mholt commented Feb 24, 2023

stffabi commented Oct 23, 2023

[assetserver] Inject runtime/IPC into all index html files #2203

[assetserver] Inject runtime/IPC into all index html files #2203

Conversation

stffabi commented Dec 13, 2022

mholt commented Dec 13, 2022

leaanthony commented Dec 14, 2022

leaanthony left a comment

Choose a reason for hiding this comment

mholt commented Dec 20, 2022 • edited

mholt commented Dec 21, 2022

stffabi commented Dec 21, 2022

mholt commented Dec 21, 2022

stffabi commented Dec 21, 2022

mholt commented Dec 21, 2022

mholt commented Dec 23, 2022 • edited

leaanthony commented Dec 23, 2022

mholt commented Dec 23, 2022

leaanthony commented Dec 23, 2022

stffabi commented Dec 23, 2022

stffabi commented Dec 23, 2022

leaanthony commented Dec 23, 2022

leaanthony commented Feb 23, 2023

mholt commented Feb 24, 2023

stffabi commented Oct 23, 2023

mholt commented Dec 20, 2022 •

edited

mholt commented Dec 23, 2022 •

edited