Return content of /index.html instead of 404 #117

hgzimmerman · 2018-07-15T21:08:44Z

The PR that implements the feature request detailed in #100.

I don't think it is exactly clean enough to merge in its current state, but I have tested it and observed that it worked as intended, returning the index.html file instead of a 404 response when a requested file path doesn't exist.

This would allow a yew application with routing to return to a routed page when a page refresh is performed, improving developer ergonomics.

koute · 2018-07-16T16:54:38Z

Thanks for the PR!

Hmm... I think it would be better to make this a little more configurable. Say, instead of this:

index-on-404 = true

something like this would be more elegant and flexible:

404-path = "/index.html"

Also, in cmd_start instead of adding extra code into the else block you could do something like this:

+ let mut artifact = last_build.deployment.get_by_url( &path );
+ if artifact.is_none() {
+     if let Some( not_found_path ) = project.404_path() {
+         artifact = last_build.deployment.get_by_url( &not_found_path )
+     }
+ }
+ if let Some( mut artifact ) = artifact {
- if let Some( mut artifact ) = last_build.deployment.get_by_url(&path) {

hgzimmerman · 2018-07-16T17:26:44Z

Thanks for the feedback!
I agree with your suggestions and will try to get something to you in the near future.

hgzimmerman · 2018-07-16T19:51:24Z

I've made the requested changes.
I have observed some strange behavior as the result of supporting the different routes.

Lets assume that I attempt to do the following with 404-path = "/index.html" set in my Web.toml, and there isn't a real index.html provided in a /static directory.

Successful load

I attempt to get http://[::1]:8000/b.
The server can't find /b so it returns index.html
index.html causes my_app.js to load.
The server finds my_app.js and app bootstraping is able to occur.

Failed load

I attempt to get http://[::1]:8000/b/a.
The server can't find /b/a so it returns index.html
index.html causes b/my_app.js to load.
The server can't find b/my_app.js and returns the content of index.html again.
What should be a script is just the content of the index.html and it fails to execute, stopping the WASM bootstrapping process.

So it can be seen that this PR allows the app to start from any 1-path-segment route, but any more segments breaks the way the browser loads subsequent resources with the current setup.

This is the html returned when http://[::1]:8000/b/a is requested:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=1" name="viewport" />
    <script>
        var Module = {};
        var __cargo_web = {};
        Object.defineProperty( Module, 'canvas', {
            get: function() {
                if( __cargo_web.canvas ) {
                    return __cargo_web.canvas;
                }

                var canvas = document.createElement( 'canvas' );
                document.querySelector( 'body' ).appendChild( canvas );
                __cargo_web.canvas = canvas;

                return canvas;
            }
        });
    </script>
</head>
<body>
    <script src="my_app.js"></script>
</body>
</html>

I think this problem can be fixed by changing this line to make the browser fetch it from an absolute location:

<body>
    <script src="/my_app.js"></script> <!-- Absolute path now -->
</body>

koute · 2018-07-16T20:01:30Z

Sounds good to me!

koute · 2018-07-16T20:03:23Z

src/cmd_start.rs

+        let mut artifact = last_build.deployment.get_by_url( &path );
+        if artifact.is_none() {
+            if let Some( ref not_found_path ) = path_404 {
+                println!("Artifact could not be found, but should try to get user specified path instead");


Having this print here is probably unnecessary (it'll just spam the console); you can put a debug! log instead, e.g.:

debug!( "{} not found; serving {} instead", path, path_404 );

Yeah, that was there as an accident. I had it there as a quick printf-debuging thing and removed it but didn't save before I committed. (I switched back from Clion to VS Code, and forgot that autosave was disabled when I made the commit).

Still a good idea to add the debug though.

hgzimmerman · 2018-07-16T22:40:55Z

src/wasm_runtime_standalone.js

@@ -19,13 +19,13 @@ if( typeof Rust === "undefined" ) {
        if( typeof window === "undefined" && typeof process === "object" ) {
            var fs = require( "fs" );
            var path = require( "path" );
-            var wasm_path = path.join( __dirname, "{{{wasm_filename}}}" );
+            var wasm_path = path.join( __dirname, "/{{{wasm_filename}}}" );


I haven't tested the additional / here, but I assume that the path.join() function would produce the same output regardless of the / being there or not.
Let me know if you would like me to remove this change.

hgzimmerman · 2018-07-16T22:41:38Z

I think aside from the small question above, this should be ready for another review.

koute · 2018-07-23T19:46:07Z

@hgzimmerman Sorry for the delay! Looks good to me, thanks a lot! 👍 I'll merge it in once we get the latest nightly stabilized.

jiegec · 2019-01-30T09:56:52Z

Any update on this?

koute · 2019-01-30T22:22:11Z

@jiegec I haven't got around to it yet.

The extra slash added to the runtimes here is actually a backwards incompatible change, which is why I haven't merged this yet. Once I finish getting cargo-web to process the .wasm files in a non-destructive way we'll be able to regenerate the .js file at any time, and then we can just add the slash dynamically only for the 404 replies, so we'll be able to have this.

Wodann · 2019-08-29T14:14:21Z

@koute are there any remaining blockers before this can be merged?

hgzimmerman · 2019-08-29T14:42:20Z

I'm around for the near future if you want me to get this in a mergable state again.

dancespiele · 2019-10-24T18:43:40Z

@koute will be nice to merge this feature. Now I have to execute 4 commands with cargo watch and build my own rendering server to run my project when with this PR only I would need to use cargo web. @hgzimmerman told you that he can get this in a mergable state.

hgzimmerman · 2021-04-30T23:43:19Z

With the time passed since this was started, I no longer plan to get this feature merged. Someone else is welcome to take up the mantle if they so choose.

add basic support to returning the index.html if a file isn't found

e3db5c8

support arbitray file path on 404

c79bede

koute reviewed Jul 16, 2018

View reviewed changes

hgzimmerman added 5 commits July 16, 2018 15:05

remove printf debug and unneeded type annotation

2302f90

added debug line for path_404

090e1f1

make template index.html point to an absolute path for the js

e47064f

fix debug statement

a5a51aa

make path for fetching wasm absolute

9b0597d

hgzimmerman commented Jul 16, 2018

View reviewed changes

hgzimmerman closed this Apr 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return content of /index.html instead of 404 #117

Return content of /index.html instead of 404 #117

hgzimmerman commented Jul 15, 2018

koute commented Jul 16, 2018

hgzimmerman commented Jul 16, 2018

hgzimmerman commented Jul 16, 2018

koute commented Jul 16, 2018

koute Jul 16, 2018

hgzimmerman Jul 16, 2018 •

edited

hgzimmerman Jul 16, 2018

hgzimmerman Jul 16, 2018

hgzimmerman commented Jul 16, 2018

koute commented Jul 23, 2018

jiegec commented Jan 30, 2019

koute commented Jan 30, 2019 •

edited

Wodann commented Aug 29, 2019

hgzimmerman commented Aug 29, 2019

dancespiele commented Oct 24, 2019 •

edited

hgzimmerman commented Apr 30, 2021

Return content of /index.html instead of 404 #117

Return content of /index.html instead of 404 #117

Conversation

hgzimmerman commented Jul 15, 2018

koute commented Jul 16, 2018

hgzimmerman commented Jul 16, 2018

hgzimmerman commented Jul 16, 2018

Successful load

Failed load

koute commented Jul 16, 2018

koute Jul 16, 2018

Choose a reason for hiding this comment

hgzimmerman Jul 16, 2018 • edited

Choose a reason for hiding this comment

hgzimmerman Jul 16, 2018

Choose a reason for hiding this comment

hgzimmerman Jul 16, 2018

Choose a reason for hiding this comment

hgzimmerman commented Jul 16, 2018

koute commented Jul 23, 2018

jiegec commented Jan 30, 2019

koute commented Jan 30, 2019 • edited

Wodann commented Aug 29, 2019

hgzimmerman commented Aug 29, 2019

dancespiele commented Oct 24, 2019 • edited

hgzimmerman commented Apr 30, 2021

hgzimmerman Jul 16, 2018 •

edited

koute commented Jan 30, 2019 •

edited

dancespiele commented Oct 24, 2019 •

edited