kbkk / abitia Goto Github PK

I think that the t variable is no longer on the zod object.

Right now errors are logged as error: {}

https://getpino.io/#/docs/api?id=serializers-object
https://github.com/pinojs/pino-std-serializers

Need to design a way to generate OpenAPI out of zod schemas

• show coverage post-test
• post coverage report in pr comments
• require 80% coverage

Cancelled: Even if Nest.js isn't perfect maintaing a set of abstractions over a more flexible framework like Fastify brings a lot of maintainability burden regarding DX, possible bugs, etc. I will pursue building on top of Nest.js and provide a set of good practices (to be established).

Original post:
So far Nest.js stands in the way of achieving a modular monolith application:

1. Relying on a hack to correctly register HTTP endpoints is not something I'd like to rely on in a production system. This can potentially break with a Nest.js update.

   abitia/services/monolith/src/index.ts

   Line 114 in 33b0572

   // Todo: this may break.

2. Nest's module system allows for easy inverse of control in code but not in the modules themselves. This is solved by async providers which are a nightmare to maintain and leads to bad DX.

   abitia/services/monolith/src/Core/Auth/NestJs/AccountAuthModule.ts

   Line 19 in 33b0572

   public static registerAsync(options: AccountAuthModuleAsyncOptions): DynamicModule {

3. (Personal) Not convinced of Nest as a whole.
   • The "go-to" DTO tools are class-validator and class-transformer which are type unsafe by design, not maintained very well. Since I've had real nightmare stories with those, this project has replaced them with a custom solution built on top of zod.

Suggested solution:
None yet, needs research. Most probably a DI container like tsyringe and build around a simple http framework.
Implement a service template using fastify and inversify.

Related PRs:

• Implement DI + Fastify #37
• #40
• Implement an OpenAPI specification generator

That line:
https://github.com/kbkk/abitia/blob/master/packages/zod-dto/src/OpenApi/zodTypeToOpenApi.ts#L30C12-L30C28

is leading the schema to have that error:

Structural error at components.schemas.LiveModel.required
should be an array of property names required within an object schema
Jump to line 2275

Needs design, top of the head action items:

• Request-scoped contextId/spanId with async local storage
• Instrumentation
  • Database calls
  • HTTP calls
  • Events sent

Turns out these can he handled out of the box with OpenTelemetry. #26

Currently EventHandlers register themselves from constructor:

#36

Hey, my team is currently using class-validator which I'm not a fan of it at all :) With Nest.js which means we're pretty locked on classes being available in the runtime for Nest to be happy.
I try to find a way to migrate towards zod for validation as much as I can, but the Swagger part is pretty useless for us as we're using GraphQL, so having Swagger imported even if we're not using it, is a bit wasteful.
To guarantee that Swagger stuff will never get into the runtime/compile-time no matter the setup, having it as a separate module will be nice.
Since you have rush.js setup, I guess, it should be pretty straightforward to achieve :)

Afterthoughts, I guess, with GraphQL we will have to stick to class-validator as, well, all GraphQL stuff is done using decorators, so we won't be going away from classes for DTOs/InputTypes/Models, but, anyway, having it separated is just a good thing anyway.

right now a faulty message breaks the whole app, the MikroOrmOutboxWorker should retry the message up to a specified amount of times

Main README:

• Rush.js setup instructions / quick guide
• Tree list with packages / services & links to their readme
• Roadmap (libraries to be written, decomposing to microservices)

Monolith README:

• Why start with a monolith?
• Architecture description / drawings
• Describe components under monolith's Core/ dir

General guides:

• Building a CI/CD workflow
• Cloud deployment considerations

• show coverage post-test
• post coverage report in pr comments
• require 80% coverage