GraphQL plugin #53
GraphQL plugin #53
Conversation
| } | ||
|
|
||
| self.services.websocket_client.lock().await.init(); | ||
|
|
||
| let action_to_clone = self.action_tx.as_ref().unwrap().clone(); | ||
|
|
||
| tokio::spawn(async move { | ||
| client(Some(action_to_clone)).await; | ||
| let _ = client(Some(action_to_clone)).await; |
There was a problem hiding this comment.
I think you can also do client(Some(action_to_clone)).await?; which would allow the error to surface from the result
There was a problem hiding this comment.
I'll check that out!
|
|
||
| use crate::components::component::Component; | ||
| use crate::components::handlers::HandlerMetadata; | ||
| use crate::components::home::{Home, WebSockerInternalState}; | ||
| use crate::components::home::Home; |
There was a problem hiding this comment.
This is really minor and you definitely don't have to do it, but a style guideline I follow is to prevent what they call "module-name prefixes", which essentially means repeating the module name within your type names, like components::home::Home or components::component::Component or components::handlers::HandlerMetadata. The Go community recommends against this as well. It's generally more concise and readable to use alternatives like components::Home, components::Component, or components::handlers::Metadata.
To do this, I'll often do public exports of specific types within my lib.rs or mod.rs files, like this. With things like pub use hooks::Hook; and pub use tags::Tag;, it allows me to use more concise imports like inject::Hook, and inject::Tag everywhere.
There was a problem hiding this comment.
Also, you can collapse a lot of these imports together into one statement, like this. The end result after adopting these two style guidelines would look something like this:
use crate::{
components::{Component, Home, handlers::Metadata},
services::websocket::{Client, Trace},
tui::{Event, Tui},
wss::client,
};
| pub fn handle_general_status(app: &mut Home, s: String) { | ||
| app.status_message = Some(s); | ||
| pub fn handle_general_status( | ||
| app: &mut Home, |
There was a problem hiding this comment.
The variable name is app but the actual struct is Home. That was confusing. 😅 I wanted to see what abort_handlers looked like, so I pulled up app.rs and couldn't find it.
| app.abort_handlers.iter().for_each(|handler| { | ||
| handler.abort(); | ||
| }); |
There was a problem hiding this comment.
It took me a minute to wrap my head around this, so I'll summarize here to make sure I understand. When you're showing a status message, you want to set a 5-second clear action. If the status message is changed during that time, you abort the prior clear and reset the timer to 5 seconds anew so that the new message isn't cleared out too soon?
| let thread_handler = tokio::spawn(async move { | ||
| sleep(Duration::from_millis(5000)).await; | ||
|
|
||
| sender.send(Action::ClearStatusMessage) | ||
| }); |
There was a problem hiding this comment.
I'm guessing this is a tokio::spawn independent task because there's no natural point to await the results to trigger polling and allow the future to progress?
| if graphql.is_none() { | ||
| return None; | ||
| } | ||
|
|
||
| let gql = graphql.unwrap(); |
There was a problem hiding this comment.
One thing that isn't immediately obvious - Option is a return value that exhibits the same question-mark (?) behavior that Result types do. You could potentially replace these lines with just let gql = graphql?;
Then, below you can add a question mark to the end of the let query, let operation_name, let operation_type, and let variables lines below so that you can entirely eliminate the if query.is_none() || variables.is_none() || operation_type.is_none() block and the .unwrap() calls underneath.
No description provided.