-
-
Notifications
You must be signed in to change notification settings - Fork 41
Prisma Kysely v2.0.0 #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Prisma Kysely v2.0.0 #110
Conversation
I've published this PR as |
Thank you for taking the time! I tried your implementation on a schema with a hundred tables and it works fine. ESLint complains about the use of namespaces. I want to note that they aren't strictly necessary as Prisma already forces the model names to be distinct even if they are in different schemas. Another interesting avenue of improvement might be to designate a schema as default, e.g. the |
Hey @Bathlamos, I just added a |
Hey 👋 I think that the use of import * as Public from 'path/to/public/schema/types'
import * as MySchema from 'path/to/my-schema/schema/types'
Public.Person
MySchema.Person More shorter files = better performance as seen with https://www.prisma.io/docs/orm/prisma-schema/overview/generators#output-splitting-and-importing-types. Each schema module could expose it's own export interface Database {
'my_schema.person': Person;
'my_schema.pet': Pet;
} This way, you could create a |
In general, Also, the output generated by Namespaces are often considered problematic when declared across multiple files, as that can make it harder for the TypeScript type checker to do its job efficiently. However, that doesn't seem to be the case here also. Given all of the above, I still believe using namespaces is the right approach here. |
The bottom line is: The current solution is forcing a specific (and probably niche - the team recommending not to use them and So either provide both, e.g.: type _Table = {...}
export type Table = _Table
export namespace MySchema {
export type Table = _Table
} ...or allow choosing the flavor of grouping (e.g. turn the boolean into a choice between
It's more about the size of the file, at least according to this, and what I (mis?)heard from the ORM team.
Another reason to avoid them? 😉 |
Hey @igalklebanov!
As far as I understood, that comment points out problems that happens when using namespaces across multiple files. (As the typescript team suffered that personally since TS's own codebase was structured as a few giant namespaces) It was easy to implement namespaces here because of our current architecture as we can simply reuse everything already made and wrap the result with an It also said:
We have eslint rules to disallow/enforce almost everything. Ideally, generated files should not be linted. Is there something more personal than linting rules?
DX usually means a combination of personal preference and familiarity.
I'd love to later help you by reviewing a PR adding support your alternative :)
The problem said states something that does not happens in this use-case: 1 single file with 1 model per file. I'll never be against new features but personal preferences does not seems a compelling use case since the current implementation does not seems worse ihmo. Although using As far as my knowledge goes into the type checker and compiler internals, enabling |
This is not about namespaces, but the public perception of them, prominent tooling/voices recommending against their usage (or recommending something else instead), and the majority of TypeScript developers not wanting them in their codebases as a result. Forcing them, by surprise, as the only way, is not good DX, and is fighting against the grain. No amount of cherry-picking is going to help the case for them. e.g. I'm more than OK with string enums, but the ship has sailed. Most people say "enums bad", because that's what they've been fed - no nuance. I understand that I'm in the minority, and I will never force enums on anyone as a result.
But this one is in the recommended preset of a linter that's downloaded 48,854,801 times per week. How many people care enough (about namespaces) to tinker with the recommended preset and allow them?
How many people configure their tooling properly? How many headaches will this cause?
I didn't mean to demand anything. This is just how I talk sometimes. :) |
I think namespaces are suitable here In my opinion I believe it's unfortunate that no-namespaces was a default/recommended rule for the TypeScript eslint plugin. I personally usually turn that one off. Additionally namespaces are a pretty similar representation in TypeScript of what it means to have a database schema. While separate files do represent some separation to some degree, I would say namespaces are a closer match in terms of what they semantically represent. Could the solution be that we have a |
closes #91
closes #88
closes #69
closes #96
closes #106
closes #115
closes #116
closes #117
closes #94
closes #97
closes #100
Previously, this was a PR created to introduce
groupBySchema
. However, following recent discussions on Kysely discord, this PR now is home to all work for the upcoming prisma release v2.0.0Old description
closes #91
I didn't need to fix any test after adding this feature, this means that everything kept running exactly as before when
groupBySchema
is nottrue
.We are using this plugin in a giant schema (100+ tables under 18 schemas) where some of them have the same or similar names, which is very hard to spot visually which schema an imported type comes from.
This plugin simply adds all enums and models under a
ts.ModuleDeclaration
respective to their schema and fixes types references to it. It makes it much easier to work with schemas, the same visually/architecturally as thinking about why schemas are written likeschema.model
inside SQL.EDIT: I also added
filterBySchema
, which just picks all models from defined schemas.Now generates:
Instead of putting everything globally as previously: