Full Review PR

[dcadenas]

This is just to easy the review feedback process for the whole project which is already living in main

[boreq]

Reading the docs I think package caching may not be enabled by default but seeing that runs are relatively fast right now it probably doesn't matter. May be something to look at in the future though.

[boreq]

Interesting, I've never used pnpm before, only yarn.

[boreq]

Looking at this for the first time I find it a bit weird that we post to a .well-known file and I think a normal REST approach of creating routes would be fine (e.g. /names) as well. That being said I really don't think this matters that much.

[boreq]

I am worried that we ended up with domain logic in the HTTP handlers. What I can see here is that in this HTTP handler we execute domain logic which seems to be validating data, verifying signatures, and then we even end up touching the database.

Whenever I encountered such approaches in the past they quickly proved to be hard to maintain and fragile whenever the project reached any level of complexity. From my perspective anything related to either domain logic or databases shouldn't show up in the HTTP handlers.

What if we want to execute the same code under different circumstances e.g. from CLI? It would be easy to miss some of those checks. What if we want to replace the database layer?

Looking at this I see that the code doesn't seem to be divided into layers with specific responsibility e.g.:

• domain layer which knowns nothing about what database is used
• database layer which doesn't contain any domain logic whatsoever so that it can be easily maintained and replaced
• application layer which glues the domain and database/adapters together
• http/cli handlers and other ports which just call things from the application layer

This is just an example ofc and maybe my brain is too hardwired to follow things like hex architecture but at least some clear domain code which is database agnostic and can be called from various places is good to see. There are many different approaches to how to layer the code in web apps like this. Again, maybe it doesn't matter but this code lights up red lights in my head.

[boreq]

Also all of this follows the imperative validateData(data) structure of calls instead of much more safe approach of simply making invalid state impossible to exist in memory by using constructors.

For example instead of doing something like:

let event = extractEvent(request);
if !eventValid(event) {
    // error
}

saveEvent(event);

A much more safe approach which makes omitting validation impossible is to create a strong type which then gets passed along with some validations in the constructor. I don't understand how to do this in JS but surely it is possible. I think classes with constructors are a thing now?

let validEvent = new ValidEvent(request);
saveEvent(validEvent);


func constructorOfValidEventButUnfortunatelyFilipDoesNotKnowJavaScipt(request) {
// unpack request
// validate data
// return strong type "ValidEvent"
}

[boreq]

I would say that all of this boils down to the term "Domain Driven Design" basically.