-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
Add a convenient dropdown to website for good examples
My vscode did something to the .css file, I don't know why :/ |
if first_render { | ||
let handle = { | ||
let link = ctx.link().clone(); | ||
Timeout::new(1, move || link.send_message(Msg::FirstLazy)) | ||
}; | ||
handle.forget(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hacky...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we do all the work of loading examples up-front and avoid the indirection? Is it slow enough to block the page load or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the chance, I think 50% of the page load is on deserializing the .yml
let tests: Vec<TestCase> = | ||
serde_yaml::from_reader(BufReader::new(File::open("../data/tests.yml").unwrap())) | ||
.expect("invalid yaml in data/tests.yml"); | ||
let tests: Vec<TestCase> = tests | ||
.into_iter() | ||
.filter(|test| test.test_enabled()) | ||
.collect(); | ||
let tests: Vec<TestCase> = serde_yaml::from_str(include_str!("../../data/tests.yml")) | ||
.expect("invalid yaml in data/tests.yml"); | ||
let tests: Vec<TestCase> = tests.into_iter().filter(TestCase::test_enabled).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we now include the tests at compile time (File::open not supported in wasm)
downside: every test change triggers a recompile.
necessary evil?
I could make a crate dedicated to this, or have platform specific implementation, but both seem worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as this is feature-gated, seems fine to me. Test changes triggering a recompile really is necessary for the web app.
FWIW, there are even crates like https://github.com/Michael-F-Bryan/include_dir/ to shove more stuff into WASM at compile-time. I... have definitely included about 500MB of binary files before. :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me, minus the necessity of lazily loading examples.
I'm less able to try this locally; main dev machine currently broken, and cargo install trunk
very bizarrely broke...
pub enum Msg { | ||
Up(Box<AppMsg>), | ||
FirstLazy, | ||
Example(String), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking my understanding here... Up
refers to the parent component, FirstLazy
is a trick to lazily load up test examples, and Example
is when a particular example has been set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, naming is hard
if first_render { | ||
let handle = { | ||
let link = ctx.link().clone(); | ||
Timeout::new(1, move || link.send_message(Msg::FirstLazy)) | ||
}; | ||
handle.forget(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we do all the work of loading examples up-front and avoid the indirection? Is it slow enough to block the page load or something?
let tests: Vec<TestCase> = | ||
serde_yaml::from_reader(BufReader::new(File::open("../data/tests.yml").unwrap())) | ||
.expect("invalid yaml in data/tests.yml"); | ||
let tests: Vec<TestCase> = tests | ||
.into_iter() | ||
.filter(|test| test.test_enabled()) | ||
.collect(); | ||
let tests: Vec<TestCase> = serde_yaml::from_str(include_str!("../../data/tests.yml")) | ||
.expect("invalid yaml in data/tests.yml"); | ||
let tests: Vec<TestCase> = tests.into_iter().filter(TestCase::test_enabled).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as this is feature-gated, seems fine to me. Test changes triggering a recompile really is necessary for the web app.
FWIW, there are even crates like https://github.com/Michael-F-Bryan/include_dir/ to shove more stuff into WASM at compile-time. I... have definitely included about 500MB of binary files before. :D
Co-authored-by: Dustin Carlino <[email protected]>
You can really see how the yaml deserialization consumes about 1.1s of load time... |
Grade A, nice! Deferring the YAML load makes perfect sense to me then. By the way, if you happen to be familiar with how to handle this kind of problem in web browsers, do you know of the "proper" way to load/parse large blobs of data without freezing things up? Downloading a large file and showing a progress bar is possible to do async by fetching in chunks. But parsing it blocks the CPU. Do any/many web apps have a nicer UX? The only two ideas to workaround are to try web workers (which don't seem to be easy to manage from Rust, though I've spotted some more recent crates that maybe help) or to use some kind of streaming serialization that can limit itself to a real deadline, yield, and let the caller update a progress bar or give the user control to interact with the page. For the latter idea, serde doesn't seem to help directly. But if the large blob is a list of stuff, then one approach is to just try and deserialize one item at a time. I haven't seen any real examples doing this though... (All of that's an aside; this PR looks fine to me -- great new feature) |
I'm not an expert, basically a no-stack developer... I do think webworkers are the way, I was just waiting for good integration with trunk... trunk-rs/trunk#285 Oh it's done. OK, maybe I will try to make a web-worker as an example then... |
looks simple enough :) |
Still OK after: https://gtmetrix.com/reports/a-b-street.github.io/mX4s9uCK/ |
Add a convenient dropdown to website for good examples