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

Migrate Application store to the global/static one #447

Merged
merged 10 commits into from
Jul 1, 2024
Merged

Conversation

roll
Copy link
Collaborator

@roll roll commented Jun 28, 2024


Historically, the application was started as a set of independent components (to be shared for Frictionless Universe), and the modular pattern was used consistently for even high-level components like applications/controllers. Now we have some time to refactor it, so using a global/static store will bring many pros:

  • state isolation behind actions
  • simplified action writing (plain functions)
  • ability to group actions by function (perceivable file lenghts)
  • reduction duplication between application/controllers (in the following PR, I will merge controllers into the global store)
  • proper debugging with time travel
  • isolated store testing
Untitled_.Jun.28.2024.11_13.AM.webm

It's a massive change that would be totally dangerous to do in JavaScript without tests in-place but in TypeScripe we can be 98% sure that it's properly migrated as it won't pass type checking otherwise.

Anyway, I added an example of testing for the store -- it will be good to write tests ASAP.

@roll roll changed the title Migrate Application store to the global one Migrate Application store to the global/static one Jun 28, 2024
@roll roll requested a review from pdelboca June 28, 2024 10:24
@roll
Copy link
Collaborator Author

roll commented Jun 28, 2024

Interestingly, why rollup is not catching the import config from Vite. I'll investigate. Upd. the vite configs are different (desktop/client)

Copy link

cloudflare-workers-and-pages bot commented Jun 28, 2024

Deploying opendataeditor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 140ee94
Status: ✅  Deploy successful!
Preview URL: https://e4a5743f.opendataeditor.pages.dev
Branch Preview URL: https://430-consolidate-stores.opendataeditor.pages.dev

View logs

@pdelboca
Copy link
Member

pdelboca commented Jul 1, 2024

Looks good @roll ! A lot cleaner :)

Agree that we need to have tests ASAP. Now that we are finishing the workflows we could start implementing them.

@roll
Copy link
Collaborator Author

roll commented Jul 1, 2024

Thanks! I use this approach in new projects, and I think it's more readable/maintainable compared to the default Zustand patterns. Also immer allows stop thinking about the way state was changed (like creating new copies of array/object to trigger re-renders) and just program the changes itself

@roll roll merged commit eb9b6d7 into main Jul 1, 2024
8 checks passed
@roll roll deleted the 430/consolidate-stores branch July 1, 2024 08:08
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.

Setup Zustand debugger
2 participants