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

Major cleanup #302

Closed
Jeffrey-Zutt opened this issue Aug 6, 2020 · 1 comment · Fixed by #300
Closed

Major cleanup #302

Jeffrey-Zutt opened this issue Aug 6, 2020 · 1 comment · Fixed by #300
Assignees
Milestone

Comments

@Jeffrey-Zutt
Copy link
Contributor

MAJOR CLEANUP

Decided to execute a major cleanup after I had to modify a lot of boilerplate code (managing state) to implement the new feature "visit wizard", which I think will only increase whenever we add more features to TOP.

Hope you're not too overwhelmed by it :-).

How to check:

Please checkout "feature/major-clean-up", run a npm i, and then npm run start to review.

Pros

1. Folder structure same as ZaakSysteem

  • Easy to reuse concepts in both repositories
  • One way of working.
  • Split code into features:
    • cases
    • itineraries
    • login
    • settings
    • shared
    • visits

2. Typesafe routes

Same solution as Zaaksysteem - frontend

3. Less code

~25% less lines of code in ts and tsx files. (Approx. 2000 Lines of code less)

4. Use generated types from Swagger schema.

As much as possible. (Not all responses are documented correctly).
When the backend is ready, switching to it completely is not that hard anymore.

5 Reusable ItineraryItemCard/ItineraryItemCardList

  • Introduced a generic ItineraryItemCard/ItineraryItemCardList used by:
    • itineraries
    • suggestions
    • open-issues
    • start-address

6 Introduced environment variables

No runtime checks anymore

7 Replaced useGlobalState with useApiRequest

  • Added options for useApiRequest:
    - lazy (boolean): When true: don't fetch data on mount.
    - example: useItineraries({ lazy: true })

  • Added options for execDelete, execPost, execPut, execPatch:

    • skipCacheClear (boolean): When true, cache won't get cleared
      - example: const { execPatch } = useItineraries({ lazy: true }); execPatch({ skipCacheClear: true })
    • useResponseAsCache (boolean). When true, cache will be populated with the response of this request.
      - example: const { execPatch } = useItineraries({ lazy: true }); execPatch({ useResponseAsCache: true })
  • Added posibility to update cache manually: eg:

const { updateCache } = useItineraries(); updateCache(( itineraries ) => {
// do something with itineraries 
})
  • Easier to reason with, as latest API response is automatically fetched and used, there is no need to update state manually.

Cons:

  • LOTS of changes
  • Spinner is shown shortly when adding/deleting an item to a list. (Because cache is being cleared, and we refetch again automatically).
    However, dragging items around wont trigger a spinner as we can safely update the cache here.
@Jeffrey-Zutt Jeffrey-Zutt linked a pull request Aug 6, 2020 that will close this issue
@peterboer
Copy link

we vragen vincent om deze check te doen, @gilleswittenberg zet jij de bugs die je nu al vond op Github?

@peterboer peterboer mentioned this issue Aug 18, 2020
6 tasks
@peterboer peterboer added this to the Sprint 5 milestone Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants