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 adapt #51

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

State adapt #51

wants to merge 7 commits into from

Conversation

mfp22
Copy link

@mfp22 mfp22 commented Jul 20, 2023

Branched off of #50

But this should build because it removes Akita. Unless something else is wrong.

I originally didn't plan on creating a PR, so I have some changes in here you may not like. I got rid of _ variable prefixes, for example.

There were a few places I needed to synchronously access state, which I handled with a quick hack using take(1)).subscribe(. For me to get rid of these, I would have to refactor the components too, not just the stores. This could be a fun project, making the whole thing 100% reactive. I could use signals too, where I think it makes sense.

For now I am just updating with these PRs over in my fork, so I thought I'd share in case you wanted anything.

@mfp22
Copy link
Author

mfp22 commented Jul 20, 2023

Oh and I had to turn on strictNullChecks for StateAdapt type inference to work correctly.

@mfp22
Copy link
Author

mfp22 commented Jul 20, 2023

Here's a YouTube video I made while refactoring this https://www.youtube.com/watch?v=sUfahKhzZzc

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.

1 participant