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

State is not very type safe #6

Closed
wilfreddenton opened this issue Apr 4, 2023 · 2 comments
Closed

State is not very type safe #6

wilfreddenton opened this issue Apr 4, 2023 · 2 comments

Comments

@wilfreddenton
Copy link

As i stated in my other issue [#5] I'm very much enjoying using this library. Thanks again!

I would love to have some more type safety around state. If I decide to change the type of some piece of state, the compiler isn't going to complain and I will hit runtime errors. If state were instead a generic type then I could fill that in with the state type for my app and run into these errors at compile time.

@cjhopman
Copy link
Contributor

cjhopman commented Apr 4, 2023

Glad to hear you're enjoying the library!

In buck we've also struggled with the lack of type safety in state, I think changing it to be a generic type provided by the app would be a great change. I expect making that change would parameterize all the places that would need to be parameterized to support the user specifying the error type as well.

This was referenced Apr 6, 2023
facebook-github-bot pushed a commit to facebook/buck2 that referenced this issue Apr 13, 2023
Summary:
Now all the state is passed explicitly to the components. State is no longer used.

State is inconvenient because it is not statically typechecked, so it is fragile, especially for components which requests different parts of state conditionally.

This diff stack addresses the issue facebookincubator/superconsole#6

Next step would be removal of the state from `superconsole` crate, but before that, other users of `superconsole` need to be migrated.

Reviewed By: themarwhal

Differential Revision: D44815383

fbshipit-source-id: 5cd8caea3a9663ff8dfd171921167e4c24915d52
@stepancheg
Copy link
Contributor

New version of superconsole has no State: d586d6f.

API changed. Now SuperConsole::new(..) no longer grabs the component to render. Instead, super_console.render(..) takes the component reference. And that component can grab whatever it wants.

Like this:

let super_console = SuperConsole::new();

for i in 0..100 {
  super_console.render(&MyComponent {
    counter: i,
  });
}

benbrittain pushed a commit to benbrittain/buck2 that referenced this issue Apr 23, 2023
Summary:
Now all the state is passed explicitly to the components. State is no longer used.

State is inconvenient because it is not statically typechecked, so it is fragile, especially for components which requests different parts of state conditionally.

This diff stack addresses the issue facebookincubator/superconsole#6

Next step would be removal of the state from `superconsole` crate, but before that, other users of `superconsole` need to be migrated.

Reviewed By: themarwhal

Differential Revision: D44815383

fbshipit-source-id: 5cd8caea3a9663ff8dfd171921167e4c24915d52
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

No branches or pull requests

3 participants