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

Update yew-router examples, make them easier to run #1401

Closed

Conversation

philip-peterson
Copy link
Contributor

@philip-peterson philip-peterson commented Jul 10, 2020

Description

Tries to make the yew-router examples more self-explanatory, easier to run by adding a run.sh in each example which launches a simple Python server. Also noticed that cargo web seems to not be working these days, so replaced those instructions with wasm-pack.

Checklist:

  • I have run ./ci/run_stable_checks.sh
  • I have reviewed my own code
  • [NA] I have added tests

@@ -2,7 +2,7 @@ use actix_files::NamedFile;
use actix_web::{get, middleware, web, App, Error, HttpResponse, HttpServer};

// You will need to change this if you use this as a template for your application.
const ASSETS_DIR: &str = "../../static";
const ASSETS_DIR: &str = "../../_static";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason I renamed this is to make it stand out; static when it's sorted alphabetically with the rest of the examples, looks like it's the name of an example.


Using the router in its expected use case (not fragment routing) requires that the server respond to requests for
resources at URLs that are routes within the router with the index.html of the application.

### Running with Python
In applicable directories, launch the `run.sh` shell script.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In applicable directories, launch the `run.sh` shell script.
In applicable directories, run the `run.sh` shell script.

yew-router/examples/README.md Outdated Show resolved Hide resolved
yew-router/examples/README.md Outdated Show resolved Hide resolved
yew-router/examples/README.md Outdated Show resolved Hide resolved
yew-router/examples/README.md Outdated Show resolved Hide resolved
yew-router/examples/minimal/run.sh Outdated Show resolved Hide resolved
. \
&& cd ../_static \
&& python3 ../start_example_server.py "$INITIAL_URL"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
)
)

@philip-peterson
Copy link
Contributor Author

@teymour-aldridge thanks for the quick review!

@teymour-aldridge
Copy link
Contributor

No problem! One thing is that wasm-pack won't work if the examples in question use stdweb.

@teymour-aldridge
Copy link
Contributor

I now see why the examples aren't working with cargo web – it's because they're using yew with web_sys (which is the new default) rather than stdweb (which used to, but is no longer, the default). cargo web is specific to stdweb and doesn't work with web_sys.

@philip-peterson
Copy link
Contributor Author

Ahh okay. Maybe we can update that again down the road? For the record, this is the error message:

Error loading Rust wasm module 'minimal_router': TypeError: "import object field '__wbindgen_placeholder__' is not an Object"

@teymour-aldridge
Copy link
Contributor

That's a lovely error message!
We moved away from stdweb because it hasn't had any development activity in over half a year and I therefore doubt that we're going to move back to it (because maintaining a fork would consume a lot of project bandwidth and web_sys is supported by the rustwasm working group).

@philip-peterson
Copy link
Contributor Author

That makes sense. Do we still need features = ["web_sys"]? It is a default feature, although we have default-features = false specified sometimes for some reason.

hgzimmerman
hgzimmerman previously approved these changes Jul 10, 2020
@hgzimmerman
Copy link
Member

The #[default-features = false] is there to drive the point home that you don't need all the features enabled if you don't intend to use them (minimal and switch) example.

Come to think of it, the switch example really isn't an example and is more of a mediocre/manual test suite. Its not meant to be ran in the browser.
I think it should be converted into a test/ directory instead - but that's outside of the scope of this PR.

@teymour-aldridge Are tests for this project expected to be ran with cargo-web still? I'm under the impression that there has been an effort to migrate them to use something else, but I haven't followed that development super closely.

@mergify mergify bot dismissed hgzimmerman’s stale review July 12, 2020 04:57

Pull request has been modified.

@siku2 siku2 added the A-examples Area: The examples label Jul 22, 2020
@jstarry
Copy link
Member

jstarry commented Sep 20, 2020

Closed in favour of #1559

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-examples Area: The examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants