drpc-todo.md

Suggestions/Questions

• Suggestion: get rid of unused code

  I suggest to turn on noUnusedParameters and noUnusedLocals, then cleanup all the instances.

• Suggestion: get rid of any, as

  Need to review usages of any and EXPR as TYPE. The latter in most cases can be converted to EXPR : TYPE which has stronger guarantes.

• Suggestion: get rid of SWR

  The recent PRs start to introduce Recoil. It will replace SWR. Need to be careful not to introduce regressions around cache invalidation.

  Also review other data fecthing and see if we can migrate it to Recoil as well (seen some usages of fetch in useEffect).

• Suggestion: typed key-manager API

  Suggestion: generate TypeScript API from OpenAPI spec using https://github.com/ajaishankar/openapi-typescript-fetch cc @termina1 can we rely on key-manager having accurate and up to date openapi spec being available?

• Suggestion: typed BFF API usage

  Suggestion: use trpc.io for typed endpoints

• Suggestion: tagged template strings for SQL

  Currently Clickhouse query composition uses template strings but it is easy to introduce SQL injection that way. Using tagged template strings would allow to immediately flag a query as being safe from SQL injections.

• Suggestion: review useEffect usage

  Reivew usages of useEffect and decide if:

  • we want to keep it as-is
  • factor out into a custom hook
  • get rid of it

• Suggestion: project structure is confusing right now

  What we can do better:

  • Split code into frontend, backend, shared
  • Split code better between the landing and the dashboard
  • Keep React components code closer to relevant pages

• Questions: review fetching data from Clickhouse

  More specifically

  • Why use convertDataArrayToDatums instead of passing data from CH to browser as-is?
  • Why sort CH results in JS?

• Suggestion: instrument with open tracing

  E.g. assign an uniq request id for each user request and pass it through.

  The app uses multiple data sources, intrumenting code with open tracing will allow us to learn where the app spends most of the time of and correlate different requests across multiple subsystems for easier debugging.

@andreypopp
If I did not comment about something -- that means I agree with your action points.

1. I agree about code-generation for key-manager API and Next.js backend, but I think I will need a technical help from you.
2. Can you please speak more about tagged template strings for SQL? Any specific package?
3. Agree about replacing useEffect in some cases with custom hooks.
4. For Clickhouse data fetching I delegated data forming to backend with convertDataArrayToDatums, I'm not sure why would we need to move it on the frontend.
5. Question about "instrument with open tracing" I think is more for @Termina1 (can't tag him here somehow!)