Conversation
* DTO => data transfer object * less circular dependencies in modules * remove ISerializable interface
RvanderLaan
left a comment
RvanderLaan
left a comment
There was a problem hiding this comment.
👏 noice. Didn't really notice any functional changes, and I agree with your restructuring choices!
What might be handy: To preserve the file/import structure and enforce the right use of it, you could look into eslint rules like this one https://github.com/javierbrea/eslint-plugin-boundaries
What do you reckon is best for the order of merging PRs? Probably this one first, eh?
| clear: () => Promise<void>; | ||
| backupToFile: (path: string) => Promise<void>; | ||
| restoreFromFile: (path: string) => Promise<void>; | ||
| peekFile: (path: string) => Promise<[numTags: number, numFiles: number]>; |
There was a problem hiding this comment.
would be cool to have an API like this for all filesystem calls too, so we can implement another back-end for it when running in a web browser. Some day...
There was a problem hiding this comment.
Maybe some day soonTM? It seems like a simple change that can improve the code base quality quite a lot. At least make the goal of testable code come closer.
We could define a file IO interface and then pass an object implementing it around. In tests we could have some mock implementation that stores a map from absolute path to bytes and then a tree of paths for matching file paths.
| } | ||
|
|
||
| async peekDatabaseFile(path: string): Promise<{ numTags: number; numFiles: number }> { | ||
| async peekFile(path: string): Promise<[numTags: number, numFiles: number]> { |
There was a problem hiding this comment.
neat, I've seen them around, but didn't realise you could do named array values like this.
I took a look and I will merge this first because the other PRs are mostly unaffected. |
2 participants
Don't panic the majority of the changes just move and/or rename types. That said there are now a lot more types and interfaces.
Notable Changes
apimodule:DTOsuffix stands for data transfer object (thread safe and plain objects; no proxy or function object).ISerializableinterface: It was never used in any function signature and we have data transfer objects, which are much looser coupled.contains recursivelyand the tag has no sub tags it will be stored and loaded ascontains. This is problematic if later a user adds a sub tag as the saved search will return wrong results, making the UX worse. I plan to tackle this in the next PR by changing the data layout of serialized search criterias.ClientBaseSearchCriteria: TheConditionDTOtype is generic over the entity but our search criteria objects do not need to be. We only use it for files and it is very unlikely to happen for any other entity. Without generics the code becomes simpler.Messagingfile: Honestly, those build errors always make me paranoid if we import something into the wrong process. So, I split the file in the hopes of less weird build errors.Ignorable Changes