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

Fix and use module UseState #98

Closed
wants to merge 2 commits into from

Conversation

Philipp-M
Copy link
Contributor

This fixes the UseState module to be compilable and usable again with the current xilem architecture.

A port to xilem_core should be straightforward (and could be done in a follow-up PR).

@@ -103,3 +108,12 @@ where
a
}
}

pub fn use_state<T, A, S, V, FInit, F>(f_init: FInit, f: F) -> UseState<T, A, S, V, FInit, F>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a doc comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I would understand exactly what UseState does, I'd do ^^ (see zulip thread). I think probably someone else should document the code (maybe in another PR), this function just runs UseState::new obviously, I added the trait bounds, so that it's easier to see in rust-analyzer what is required to get this into a View (as it otherwise doesn't make much sense I think).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment, could probably be better, but I guess better than nothing for now.

@DJMcNab
Copy link
Member

DJMcNab commented Sep 13, 2024

I think we can close this for now. use_state will likely eventually be needed, but we're trying to avoid that for as long as possible...

@Philipp-M
Copy link
Contributor Author

Yeah lets close this, I don't think we should rely on the app state to be Arc<State> anyway, as this severely restricts the use-cases.
I do hope we find ways to make adapt/map_state/lens more flexible (e.g. modularice distinct state items, as described here), so that we can keep the all the user-state in AppState

@Philipp-M Philipp-M closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants