TTC
Theo's Typesafe Cult•2y ago
Rhys

Prisma feel like it leads to bad code design patterns

I’ve been learning and working with Prisma as I rewrite a project of mine in Typescript and one thing really stood out to me with it, it’s really easy to create complex queries and create / modify data from multiple places Quick example, users can change settings in my project, that has the potential to happen from multiple places and if I made each one of those it’s own Prisma call with update, then if I ever wanted to add analytics to settings changes I’d have to go to each place and put an event listener The way I’ve set up Prisma in my project is similar to how Discord JS handles their interactions, with managers and structures If someone wants to update a user setting, that has to go through the user setting manager, this way there’s one single edit function so if I ever go to dispatch events on user settings change, I can be sure I’ll capture everything Along with that, setting it up this way allows me to enforce rules on the codebase, for example no creating of foreign keys in Prisma queries, if you want to make an object you have to go through the manager to ensure all relevant events are fired The reason I ask this is because I’m writing a lot of boilerplate on top of Prisma now to try to make in this style, while I’m somewhat happy with where I’m at with it, it also feels slightly wrong. I’m a bit torn on if this is dramatically over complicating things or a good approach for scaling my application. I think for my project, my approach makes sense, with the trade off being the complexity of Prisma for a setup that I know will be scalable How do you approach Prisma in your projects, do you have help functions, something more complex, or just raw queries throughout the code? Here’s some code to help understand what I’m talking about, it’s a bit rough around the edges, just playing with ideas for it atm Abstract classes: Manager: https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/managers/manager.ts Structure: https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/structures/base.ts Implementation: Channel settings manager: https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/managers/channel-settings/channel-settings-manager.ts Channel settings structure: https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/structures/channel-settings.ts
33 Replies
Rhys
Rhys•2y ago
I realize that’s a long wall of text to quick TL;DR of the concerns: 1. Queries get scattered throughout the codebase 2. The complexity of queries allows for data to be created / destroyed / updated in multiple locations resulting in possibly missing events 3. Wrapping Prisma queries results in a bunch of boilerplate and loses the power that Prisma offers
☘
☘•2y ago
check remix planner by Ryan Florence
Unknown User
Unknown User•2y ago
Message Not Public
Sign In & Join Server To View
Rhys
Rhys•2y ago
This is interesting, it looks like a query per function which could be a good approach Yeah I suppose with Prisma it's up to you to use the power to choose your own adventure with it, in my case, I'm choosing to trade off the power of having a bunch of complex queries in exchange for having more control over how data flows in my application as that's what is important to me - thanks for your input on it In other things I'm doing, now using custom types made out of my Prisma types with Pick & Partial to ensure that callers can't modify certain fields 
export type UserCreateArgs = Pick<User, "id" | "name"> & Partial<Pick<User, "created_at" | "email">>;
export type UserUpdateArgs = Partial<Pick<User, "avatar" | "name" | "created_at" | "email">>;
export type UserCreateArgs = Pick<User, "id" | "name"> & Partial<Pick<User, "created_at" | "email">>;
export type UserUpdateArgs = Partial<Pick<User, "avatar" | "name" | "created_at" | "email">>;
This setup feels both pretty wrong and right, for my project there's likely going to be a lot of contributors who aren't familiar with the codebase, so this tradeoff again increases safety imo The only drawback is its so much code to write for the managers, it's helped a bit with generics & inheritance but still
robotkutya
robotkutya•2y ago
it's always a game of whack-a-mole... you reduce the complexity for reasons here, then suddenly some new complexity pops up there... the right answer depends a lot on scale scale of data, scale of concurrency, scale of number of people working on it, scale of organizational stupidity (that's a big one usually) hey , I checked out your code just some first impressions / food for thought you are doing a kind-of mvc-ish approach I'd consider a more flat structure with more co-location, organized around entities / features, instead of (deeply nested) organization around functions so instead of having core/src/managers/channel-settings/channel-settings-manager.ts you could have core/src so far so good, because you're using turbo repo and packages, etc and then features/channels
Rhys
Rhys•2y ago
Thanks for the feedback on that I appreciate it, do you have a repo you recommend I check out as a reference? I'm playing around with the structure of it atm, The main thing I like about the current structure is it lets me quickly find and view a function as I can do Ctrl + P in VSCode and open the specific file for say creating a user, making my flow to editing code be: realize a function need to be changed->open file and edit instead of realize change is needed->open file-> scroll to find function->edit
Unknown User
Unknown User•2y ago
Message Not Public
Sign In & Join Server To View
Rhys
Rhys•2y ago
That’s a pretty interesting approach, took me quite a few times of reading over your message to understand it 😅 These concepts seem like repeatable problems that would show up in multiple different codebases, I’m surprised this isn’t a more solved issue unless I’m missing something
robotkutya
robotkutya•2y ago
you could have the same with a more flat structure I also like the (i'm on a mac) CMD + T shortcut, which jumps right into the function definition / type definition, etc
Rhys
Rhys•2y ago
With a flat structure, does it result in files ballooning in line count? That's a slight concern of mine as when you load up a file thats >500 lines it takes a long time to digest it Quick example, I currently have this as it's own file get.ts 
import { PrismaClient } from "@prisma/client";
import { EBase, DatabaseCommand, Cache } from "./manager";

export interface FindUnique<Command extends GetUniqueCommand<any, any>> {
  // eslint-disable-next-line no-unused-vars
  get(args: Command["where"]): ReturnType<Command["execute"]>;
}

export interface FindMany<Command extends GetManyCommand<any, any>> {
  // eslint-disable-next-line no-unused-vars
  findMany(args: Command["where"]): ReturnType<Command["execute"]>;
}

export abstract class GetCommand<T extends EBase, GetArgs, GetType extends T | T[]>
  implements DatabaseCommand<T>
{
  protected abstract getFromCache(): GetType | null;
  protected abstract fetch(): Promise<GetType | null>;
  // eslint-disable-next-line no-unused-vars
  protected abstract updateCache(new_value: GetType): void;

  public abstract execute(): Promise<GetType | null>;
  constructor(
    // eslint-disable-next-line no-unused-vars
    public readonly cache: Cache<T>,
    // eslint-disable-next-line no-unused-vars
    public readonly prisma: PrismaClient,
    // eslint-disable-next-line no-unused-vars
    public readonly where: GetArgs
  ) {}
}

export abstract class GetUniqueCommand<T extends EBase, GetArgs> extends GetCommand<T, GetArgs, T> {
  public async execute(): Promise<T | null> {
    const data = this.getFromCache();
    if (data) return data;

    const fetched = await this.fetch();
    if (fetched == null) {
      return null;
    }
    this.updateCache(fetched);
    return fetched;
  }
}

export abstract class GetManyCommand<T extends EBase, GetManyArgs> extends GetCommand<
  T,
  GetManyArgs,
  T[]
> {
  public async execute(): Promise<T[]> {
    const fetched = await this.fetch();
    if (fetched == null) {
      return [];
    }
    this.updateCache(fetched);
    return fetched;
  }
}
import { PrismaClient } from "@prisma/client";
import { EBase, DatabaseCommand, Cache } from "./manager";

export interface FindUnique<Command extends GetUniqueCommand<any, any>> {
  // eslint-disable-next-line no-unused-vars
  get(args: Command["where"]): ReturnType<Command["execute"]>;
}

export interface FindMany<Command extends GetManyCommand<any, any>> {
  // eslint-disable-next-line no-unused-vars
  findMany(args: Command["where"]): ReturnType<Command["execute"]>;
}

export abstract class GetCommand<T extends EBase, GetArgs, GetType extends T | T[]>
  implements DatabaseCommand<T>
{
  protected abstract getFromCache(): GetType | null;
  protected abstract fetch(): Promise<GetType | null>;
  // eslint-disable-next-line no-unused-vars
  protected abstract updateCache(new_value: GetType): void;

  public abstract execute(): Promise<GetType | null>;
  constructor(
    // eslint-disable-next-line no-unused-vars
    public readonly cache: Cache<T>,
    // eslint-disable-next-line no-unused-vars
    public readonly prisma: PrismaClient,
    // eslint-disable-next-line no-unused-vars
    public readonly where: GetArgs
  ) {}
}

export abstract class GetUniqueCommand<T extends EBase, GetArgs> extends GetCommand<T, GetArgs, T> {
  public async execute(): Promise<T | null> {
    const data = this.getFromCache();
    if (data) return data;

    const fetched = await this.fetch();
    if (fetched == null) {
      return null;
    }
    this.updateCache(fetched);
    return fetched;
  }
}

export abstract class GetManyCommand<T extends EBase, GetManyArgs> extends GetCommand<
  T,
  GetManyArgs,
  T[]
> {
  public async execute(): Promise<T[]> {
    const fetched = await this.fetch();
    if (fetched == null) {
      return [];
    }
    this.updateCache(fetched);
    return fetched;
  }
}
In a flat structure, would I be able to make get.ts, update.ts, delete.ts... etc or would I just make a crud.ts file and put it all in there? I could do something maybe like this also 
src
└───managers
│   └───channel-settings
│       │   create.ts
│       │   update.ts
│       │   manager.ts
src
└───managers
│   └───channel-settings
│       │   create.ts
│       │   update.ts
│       │   manager.ts
if you've got a good example repo I'd love to take a look at an example to get a better idea of the structure
Unknown User
Unknown User•2y ago
Message Not Public
Sign In & Join Server To View
robotkutya
robotkutya•2y ago
I would organize it around entities, so user.ts --> would have all of the crud operations and if you want to have these abstract classes (I would probably avoid them and lean towards a functional style, but that's a different conversation), I would simply put those inside a utils.ts if that gets out of hand, then you can introduce another layer, and have the entities as folders, inside said foders maybe start out with users/crud.ts ... users/utils.ts then if that's not granular enough, you can break it down even more, also what @natefrosty21 said about service.ts ... api.ts and so on can make sense There's really no right or wrong answer, you should experiment with this, and your approach should change and evolve with your project. It is a good strategy to start as flat as you possibly can and add abstractions very conservatively. The problem is that once you committed to an abstraction, it's generally harder to make changes. To me it seems like in your project you barely have any code yet, and are worrying about how to organize that code. It doesn't really matter 🙂 Grow the project and if you run into some noticeable / measurable problem, THEN worry about what abstractions you have to use. this looks really interesting, I don't see how it would help until a fairly large scale, but maybe I just need to play around with it a bit do you have any hands on experience with it?
Rhys
Rhys•2y ago
I’m sort of at that stage now 😅 - I’m refactoring an existing project with like 20-25k lines of Python and trying to avoid making the same mistakes that I did on that project I found that as the codebase scaled it became harder to do things like add analytics, ensure that foregin keys properly exist before creating objects, reuse code, etc Trying to do this version of it a bit differently where I focus more on design patterns and good architecture at the start as last time not doing that really hurt my progress That’s kinda why I’m so focused on these abstractions now as I’m trying to solve the problems I ran into on my last codebase and I think the best way to solve it is through this manager path, just a matter of figuring out how to put it together I’ve never seen this before that’s pretty interesting, the cache is something to just have in memory as the code is also being used on a discord bot, so if it get spammed with requests the goal is to not refetch that data if needed
robotkutya
robotkutya•2y ago
ah, gotcha, makes sense then
Rhys
Rhys•2y ago
I think I'm sort of close to the design I want for it, I've got an idea on how to cut down on a lot of the boilerplate, I'll post that in this thread when it's ready
robotkutya
robotkutya•2y ago
If you come from the Python world (I do too btw), then there are a few common patterns that might be a good idea to "unlearn" When python and Django had it's golden era, in general OOP was much more the norm than today. So you obviously represented your data as classes, and then it just made sense to encapsulate your transformations as instance / class methods with your data then like you set up your database connection as a singleton class (and you remember that from school anyways) and it all seemed to just fit nicely togetehr but, it just doesn't scale well
robotkutya
robotkutya•2y ago
robotkutya
robotkutya•2y ago
it really does feel like it should work, the abstractions are very pretty, elegant but it just doesn't SO you got used to solving everything with like abstract classes, decorators Base classes... factories.... design pattern this, design pattern that A much better model is to think of your application as a series of transformations and side-effects on your data. And you pipe these together, one after the other. You can think of stuff like "adding analytics" as just another step in the pipeline A specific "transformation", where you don't actually change anything on the data, you fire some side-effects
robotkutya
robotkutya•2y ago
With your current implementation, you have: 1) a base class https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/structures/base.ts 2) an abstract class for managers that extends the base class https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/managers/manager.ts 3) an entity specific manager that implements the abstract class https://github.com/AnswerOverflow/AnswerOverflow/blob/Turbo-Repo-Base/packages/core/src/managers/users/user-manager.ts Contrast this to what you are doing, which is: 1) crud 2) caching for the R in crud
GitHub
AnswerOverflow/base.ts at Turbo-Repo-Base · AnswerOverflow/AnswerOv...
Contribute to AnswerOverflow/AnswerOverflow development by creating an account on GitHub.
GitHub
AnswerOverflow/manager.ts at Turbo-Repo-Base · AnswerOverflow/Answe...
Contribute to AnswerOverflow/AnswerOverflow development by creating an account on GitHub.
GitHub
AnswerOverflow/user-manager.ts at Turbo-Repo-Base · AnswerOverflow/...
Contribute to AnswerOverflow/AnswerOverflow development by creating an account on GitHub.
robotkutya
robotkutya•2y ago
I just don't see the value of all these classes... what are they for? What problems do they solve? You already have a good abstraction for the CRUD stuff --> Prisma! You want to have some sort of caching for the R part, why not just create a utility function, something like read(cache: Cache, query: Query): Result and... that's about it If you need to add some monitoring / logging / analytics, then create a side-effect for it mySideEffect(data: Data): Data and add it as an extra step in your pipe It makes sense to reuse your queries and maybe collect them in one place (I guess this is one problem the manager class sort of solves) but why not just do something like this? 
# entity.ts

export const mySpecificQuery(args: Args): Data

export const anotherQuery(args: Args): Data
# entity.ts

export const mySpecificQuery(args: Args): Data

export const anotherQuery(args: Args): Data
and you can just have a convention of always starting with CRUD and then the more exotic ones on a side note, why have the caching layer in front of the database at all? I'm a bit sceptical you'd get measurable benefits if you move it up however to the network request you get a very noticable improvement
Rhys
Rhys•2y ago
Thank you for the feedback on it, I really appreciate the time you’re putting in to help me with this
Solving everything with abstract classes, decorators, etc
Yeah I’m looking at this code again now and it’s just too complex for what it’s doing, I agree with that - for making a manger there’s too much boilerplate you have to write and it’s hard to conceptualize
Transformations and side effects
So on this note, rather than directly calling side effects I was planning on using the observer pattern in the manager, so when say an edit comes in, it’d dispatch an event for others to use The reasoning on this is I’m wanting to separate that analytics logic from the database calls, this way since it’s an open source project, if someone wants to disable analytics all that needs to happen is the analytics never subscribed to listening for the event
Already have a good abstraction for CRUD
Prisma is pretty nice, but the concern I guess is it’s almost too powerful The goal of this project I’m making is to be accessible to a bunch of people as an open source project and if I expose the raw Prisma args then new devs to the project, or me being forgetful, may end up changing fields that really shouldn’t be i.e a created at / user id field Along with that I’m wanting to prevent creation of classes from nested Prisma queries again to make sure all the side effects fire properly Also some of my data types need to be transformed before being given back to the caller, for example I want to turn a number for settings into a bitfield so the utility functions are needed for that
why have an in memory cache?
Yeah I’m getting rid of that now, trying to focus on getting this manager part right and then may revisit it but the main reasoning on it is it’s easy enough to have and isn’t too complex to implement I saw other open source projects have a similar in memory cache so I figured it’d be helpful I’ll play around with the concepts you suggested and see what I can come up with, I think I may end up doing a mix of the two
robotkutya
robotkutya•2y ago
in memory cache is good, I'd just solve the network level caching first, and then focus on the db level caching better bang for buck
Rhys
Rhys•2y ago
Main thing was in memory caching just seemed easiest to add on, im assuming something for network level caching would be like Redis or some other service?
robotkutya
robotkutya•2y ago
caching is a big topic people have been posting from these docs, so I've been browsing the past half hour lookie what I found!
robotkutya
robotkutya•2y ago
robotkutya
robotkutya•2y ago
you can achieve what the observer pattern is for via functional patterns you don't have to go OOP for that
robotkutya
robotkutya•2y ago
https://en.wikipedia.org/wiki/Functional_reactive_programming
Functional reactive programming
Functional reactive programming (FRP) is a programming paradigm for reactive programming (asynchronous dataflow programming) using the building blocks of functional programming (e.g. map, reduce, filter). FRP has been used for programming graphical user interfaces (GUIs), robotics, games, and music, aiming to simplify these problems by explicitl...
robotkutya
robotkutya•2y ago
trying to find a good example for you, but my google-foo is not up to it somehow, anyways I'd look around here https://github.com/ReactiveX/rxjs
GitHub
GitHub - ReactiveX/rxjs: A reactive programming library for JavaScript
A reactive programming library for JavaScript. Contribute to ReactiveX/rxjs development by creating an account on GitHub.
robotkutya
robotkutya•2y ago
And in regards to controlling / limiting Prisma, well simply wrapping the prisma queries into functions seem to solve that, no? E.g. 
createUser(createData: CreateUser): User => {
  return prisma.user.create({
    data: {
      ...createData
  })
}
createUser(createData: CreateUser): User => {
  return prisma.user.create({
    data: {
      ...createData
  })
}
and finally for the data transformation stuff, that's where FP shines! just add an extra transformation step, something like 
steps = [
  getFromDB,
  normalize,
  sanitize,
  serialize,
  complicatedCalculation,
]

getUser = pipe(steps)
steps = [
  getFromDB,
  normalize,
  sanitize,
  serialize,
  complicatedCalculation,
]

getUser = pipe(steps)
I'm writing pseudo code, but hope it makes sense anyways, go build cool sh$t champ! was just trying to show a different perspective ❤️ I think I'll shut up now 😄 actually, I kind of got interested, you mentioned this being an opensource project and it's a cool idea to open up discord to google indexing! wanna move this conversation to github and collab on this? 🙂
Rhys
Rhys•2y ago
Sure I'd be happy to! I was planning on writing the base of it first and then opening it up for collaboration but honestly the core parts of it are the most important part to get right so more help is welcome t3ggLove
robotkutya
robotkutya•2y ago
coolio! I'm going hiking on the weekend, but lets talk more next week 🖖 I'll friend you in the meantime if you don't mind as a bookmark of sorts
Rhys
Rhys•2y ago
sounds good! in the meantime I'll set up the GitHub project to be a bit more collaborator friendly the good news is the project should just work off the bat if you've got VSCode and docker installed
Unknown User
Unknown User•2y ago
Message Not Public
Sign In & Join Server To View
Want results from more Discord servers?
Add your server