prisma: [Schema] Please rethink model block syntax
model User {
id Int @id
email String @unique
}
model User {
id Int @id
email String @unique
username String @unique
}
My biggest gripe with the current syntax is how spacing can change every single field in the model. See adding username changes spacing for other fields. This hurts blame for absolutely no reason. I don’t get why:
No pull requests with different spacing schemes.
was such a big deal.
Like gofmt and unlike prettier, there are no options for configurability here. There is exactly one way to format a prisma file.
I am not sure I buy this completely given I don’t know the drawbacks for Schema syntax in v1.34 (latest stable) and what this particular new syntax really adds.
(Early enter. Writing the post)
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Reactions: 35
- Comments: 28 (12 by maintainers)
@sapkra thanks for your thoughts. I agree that for this small datamodel, no formatting is just as readable. Plus it’s simpler.
While evaluating our decision, we’ve been looking at large datamodels that you’d find in a real app:
I think the column alignment improves readability quite a bit.
I notice a pattern here that I’ve noticed in many other areas of software development: separation of concerns.
The formatted version is sliced vertically.
The non-formatted version is sliced horizontally.
Which one is more useful when actually working in a large dataset? I would argue that horizontal organization is far more important. When glancing at a screenshot of code, columns look far more aesthetically pleasing; but that’s not how we actually read our data when editing, adding, or debugging.
We don’t vertically scan columns of types or tags. We scan field names, then look ahead horizontally to see which types and tags apply to that field. This is markedly more difficult in a column format, especially when there’s a large gap between the longest and shortest field names.
How does this relate to separation of concerns? It reminds me of organizing HTML/CSS/JS by file-type instead of by component. Each line in the model is like a component - it has a name, type, and decorators. These three values should be closely coupled together and independently organized; not spread out based on the position of values in other rows.
I definitely see how column formatting appears more readable, but in practice it is not. If I have to drag my cursor across the line or hold a ruler up to my screen just to see which type/decorators apply to a field, it’s a bad design. Not to mention the terrible diffs and merge conflicts that this causes.
@Akkuma thanks for the thoughtful feedback! We are indeed aiming for number 4. From a syntactic perspective, we’ve designed the schema where formatting is 100% optional.
We don’t have an opinion on if you should format or not, just that there should only be one way to format files. Rather than a hundred options and prismarc.json’s flying around everywhere.
For VSCode we’ll auto-format by default, but you will be able to turn that off with:
Thanks for bringing this up @kartikthapar! You bring up some very good points that I’ll try and address and then we can discuss further 😊
We spent a lot of time debating this internally and I was the one who pushed for this change. We ultimately thought that the readability of the schema outweighed the occasional “over-format”. Let me walk you through our thinking.
We first heard about this issue while talking to some Facebook employees. The reason GraphQL isn’t space-aligned is because they rely heavily on
git blametooling. This seemed like a compelling reason to ditch space-alignment. However when we probed further, we realized that they would often run codemods across their whole codebase that would also mess up theirgit blame’s.We also see very large Go projects (docker, kubernetes, terraform, google cloud) running
gofmtacross their whole codebase everyday. This suffers the same problem you described and since we haven’t heard the community complain about it, this gave us the confidence to make this call.One feature that will help reduce this “over-formatting” problem is that formatting can be “reset” with new lines. For example,
Can be re-formatted as:
With all this being said, Prisma 2 is still in preview so any decision can be walked back and we’d love to your thoughts!
This is ends up being a big deal for large engineering organizations and large open source projects.
Maintainers and project owners are already strapped for time and can’t afford to spend it requesting aesthetic changes. I learned this first-hand both working in a team of 30 at Yahoo and also managing https://github.com/cheeriojs/cheerio (before auto-formatters like prettier came along)
I’d say the main motivation for this change was that a lot of people were confusing the prisma schema with GraphQL’s schema. It makes sense because they use the same syntax, but they live on different layers of the application stack. Additionally, we wanted a single file to configure prisma and model-level attributes (for things like composite indexes).
Hope this helps! As I mentioned, during the Preview period every decision we’ve made so far can be revisited and I look forward to hearing your perspective!
Thanks @matthewmueller for addressing this.
Question:
Are you saying that adding a new line with a larger length variable will not auto-reset all the other lines to accommodate? If so, I am happy with that as that will solve the issue as I understand it and you can ignore the rest of this comment.
Now some other thoughts.
I have seen different sides of this in my (small) engineering experience. However, I don’t consider this as a preference problem - I see this as a tooling problem. At Facebook for example we have 2-space indents and Instagram we use 4 (uh). It is what it is. Linter, tooling will take care of it before committing code.
Now I don’t expect similar enforcement in other open source projects but I believe in strict enforcement of such patterns. If the requirement is 2 spaces - just use 2 spaces or your code is rejected.
Re: Blame
We do use git blame/mercurial blame quite significantly but yes that blame is heavily overwritten everyday through extensive code-mods. I am unsure how Github handles blame but we can skip different commits and continue blaming. It gets much easier to read blame like:
vs
Another point I failed to mention before is conflicts. One of the biggest issues in codegen or such auto-format mechanisms is crazy conflicts in code. Just imagine two engineers working on the schema and adding fields to the same model. If they are using different length’d fields, they will always conflict. That is poor devEx.
I must say I’m a bit disappointed to find there is no way to opt-out of the aligned indentation model.
I agree that there’s a case to be made for readability but to me, git blame is an incredibly important part of readability. Based on @matthewmueller 's response (https://github.com/prisma/prisma/issues/90#issuecomment-510383292), there seems to have been some agreement on this initially:
From what I can see, this reasoning was discredited and eventually dropped “because other tools messed with git blame anyways”:
This is not a valid excuse IMO - rather it means you or your tools are doing something wrong. I have worked in projects with many developers and years of readable git blame; it most certainly is possible.
I’m complaining about it; this thread is full of people complaining about it and I’m sure there are others complaining in these other projects as well, struggling to get their voices heard in the flood of bug reports and discussions. Personally, I also think a lot of people are not using git to its full potential and may simply be unaware of the impact unrelated changes can have.
In the end, I agree with @Akkuma (https://github.com/prisma/prisma/issues/90#issuecomment-515097961) - this seems like a tabs vs spaces debate. I don’t think there will be a consensus. Therefore, I would like to highlight @dortamiguel 's underappreciated comment (https://github.com/prisma/prisma/issues/90#issuecomment-515783311) and make the same request:
Could the formatter please be extended to support an opt-in “git blame friendly” mode, where column alignment can be disabled?
Sorry to revive this old thread - this is an important feature to me.
I disagree with this pretty heavily.
With the formatting that @matthewmueller posted above, my first instinct is indeed to look for the key that i’m trying to find, but now, because everything is aligned by columns, my eye naturally moved to the exact spot where i know all of the types are, or where all of the metadata is. Without that, my eyes and brain spend more time trying to figure out exactly where it needs to look to find the data I want.
This just makes me think the code in question is over-cluttered and contains too much stuff, or that things aren’t logically clustered into sections.
please let us use git-blame…
The most future-proof solution to this would be to use tabs, and implement/encourage/wait for elastic tabstops to be implemented in various editors. This means people who want their columns aligned can have that, and those who don’t want confusing git blames or crazy merge conflicts (currently experiencing the latter because of two small changes to our schema by two members) don’t have to deal with that either. It’s really the best of every world, and the only real blocker is that editors aren’t fully on board yet.
Relevant issues: https://github.com/neovim/neovim/issues/1419 https://github.com/microsoft/vscode/issues/3932
@beeplin I agree that the current regression makes this incredibly painful. I’ve opened this issue to hopefully resolve this soon: https://github.com/prisma/prisma/issues/2487
Since this discussion is getting long, I’m going to re-iterate what I said in the beginning:
We’re not trying to do anything controversial here. We’re simply trying to follow Go’s rules for formatting. Formatting rules that have been applied to massive projects like Kubernetes, Docker, and Terraform.
Here’s an example: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L449-L460
You’ll notice that you can use comments & newlines to reset formatting. When we get this working properly, I promise that you won’t ever think about the formatting, but you’ll be happy that it’s there.
Highly recommend active line highlighting:
or with vim bindings
/searchterm [enter]I’m a fan of gofmt and universal consistent formatting so just having an option to disable the one format in whatever IDE people are using is my preference 😃
Can this be optional?
@auderer I want to reference a few things to prove that this works even in large scale applications. I think this was inspired by Go’s fmt and other languages/tools such as Terraform/HCL. They already made this decision for their language and in my opinion, it works very well. For example, if you just think about kubernetes with 80k commits and 1000+ PRs I guess you can say it’s not too bad.
Also, if you have some fields with drag other fields too much with them, you can always separate them with an extra new line, so it forms another group (see @matthewmueller’s picture and the User’s icon & id fields).
(edit: I’m not saying the formatted solution is better, I just wanted to add this here)