Skip to content
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

Improve exports of generated models to allow for intellisense / auto import #14392

Open
AndrewSouthpaw opened this issue Jul 19, 2022 · 12 comments
Labels
kind/improvement An improvement to existing feature and code. team/client Issue for team Client. topic: client types Types in Prisma Client topic: dx topic: editor

Comments

@AndrewSouthpaw
Copy link
Contributor

AndrewSouthpaw commented Jul 19, 2022

Problem

modules/gather-prisma-backend/node_modules/@prisma/client/index.d.ts does a wildcard export of the generated types:

export * from '.prisma/client'

Say I have a model like User and in my code I type User[cur] and initiate auto import, it won't find the User from @prisma/client.

If I manually type out

import { User } from '@prisma/client`

That works fine, but it's cumbersome to write out manually.

Suggested solution

Improve the @prisma/client/index.d.ts to explicitly reexport at least the model types:

export * from '.prisma/client'
export { User, Foo, Bar } from '.prisma/client' 

Alternatives

We can do this re-exporting on our end with some light scripting, but seems like a huge DX win for prisma and probably not that hard?

Additional context

I'd be happy to contribute a PR if this seems like a promising idea!

@aqrln aqrln added team/client Issue for team Client. topic: dx kind/improvement An improvement to existing feature and code. labels Jul 20, 2022
@aqrln
Copy link
Member

aqrln commented Jul 20, 2022

Overwriting the files inside the package is quite problematic, otherwise we could just generate the client inside @prisma/client. I guess in this case it might be relatively okay with popular package managers and typical project setups as the functionality won't change if/when the file is restored to its original state, however, it's still quite risky since the package may be shared between different projects in general case.

That said, this problem shouldn't occur in the first place, and I cannot reproduce it on my side — for me auto-importing models works, and I'm pretty sure it has been working before as well. To me it sounds like an issue in tsserver (assuming your editor is using a language server based on it) that occurs with your project and likely needs to be reported upstream, together with a reproduction.

Can you share more details about your setup? What editor or IDE are you using, what is your TypeScript version, etc?

@AndrewSouthpaw
Copy link
Contributor Author

AndrewSouthpaw commented Jul 20, 2022

I cannot reproduce it

Oh! 😮 Here's my setup:

Webstorm Build #WS-221.5080.193
TS 4.5.4

I can confirm I have the same issue on VS Code 1.69.2.

Thanks for the quick reply! I wonder if it's something about running on a slightly older TS version...

@aqrln
Copy link
Member

aqrln commented Jul 20, 2022

Webstorm Build #WS-221.5080.193

I was wondering, in fact, if you might be using IntelliJ IDEA or WebStorm with their static analysis engine instead of the language server (or perhaps if they could be using their own engine for autocompletion even if when the TS Language Server integration is enabled, like they do for some refactorings, AFAIK). But since you can reproduce this in VS Code as well, I think this comes from tsserver.

I wonder if it's something about running on a slightly older TS version

Won't hurt to re-check, but unless you specifically configured VS Code to use the workspace version of TypeScript, you have already checked it with TypeScript 4.7.3 which comes with VS Code 1.69.2. The latest stable version is already 4.7.4 though, so you might want to check it just in case too. There seems to be one commit related to auto-imports in 4.7.4 (microsoft/TypeScript#49442). My hunch is it's probably unrelated to your case, but might be worth checking.

Does it happen to you in this project only, or can you also reproduce it in a new/different project as well? How big is the schema file in this project (and, consequently, the generated .d.ts file)?

@janpio janpio added the topic: client types Types in Prisma Client label Jul 30, 2022
@AndrewSouthpaw
Copy link
Contributor Author

AndrewSouthpaw commented Jul 31, 2022

Thanks for the response, and sorry for the slow reply! I needed to find a time to make a repro... here it is: https://github.com/AndrewSouthpaw/prisma-import-repro-july-30.

I upgraded to TS 4.7.4, no effect there, auto imports don't work for WS.

demo1

VS Code seems to be slightly better, but only when my cursor is inside the import statement, it still isn't able to grab it on the fly.

demo1

I hunted through my WS config but didn't see anything about using the WS import system vs tsserver.

@AndrewSouthpaw
Copy link
Contributor Author

AndrewSouthpaw commented Aug 9, 2022

FWIW I worked around the issue by manually including the .prisma/client/index.d.ts file in my WebStorm directory structure.

Cursor_and_gather-town_–_gather-game-client_…_GameMap_ts

That lets me import on the fly at least with WebStorm. I haven't found a way to do it with VS Code yet.

Were you able to repro with the repo I shared?

@aqrln
Copy link
Member

aqrln commented Aug 9, 2022

@AndrewSouthpaw

Were you able to repro with the repo I shared?

Sorry, I didn't see your previous message. I just tried it with your repro and it seems to work fine for me:

video2

One thing I noticed in your recording is that you are typing the type name in a value position. I'm pretty sure that it's expected that you don't get automatic importing and automatic completion for it (even after importing it manually), that's how it always works in VS Code or any editor that uses tsserver, I believe. There's nothing specific to Prisma or how its types are organized here, you would get exactly the same behavior with any other types.

@aqrln
Copy link
Member

aqrln commented Aug 9, 2022

In other words, when you type this:

import { PrismaClient,  } from "@prisma/client";

GatherEvent // <--
// ~~~~~~~~

export const client = new PrismaClient({
  datasources: {
    db: { url: process.env.DATABASE_URL },
  },
});

It's not syntactically possible for GatherEvent to be a type here, and there's no value called GatherEvent that could be imported or autocompleted. That would work with a class or an enum (because it's both a type and a value at the same time), but not with a pure type or an interface.

I don't use WebStorm, so I don't know how it works there, it's possible that it just completes the identifiers regardless of the kind of the item an identifier refers to.

@AndrewSouthpaw
Copy link
Contributor Author

AndrewSouthpaw commented Aug 10, 2022

In my effort to do a repro, I was sloppy about code usage. 😅 I can confirm VS Code works fine for importing when using it as a type declaration, rather than an invalid string of text.

Strangely, WebStorm still doesn't work. If I add .prisma as unexcluded, then it works fine.

image

I wanted to see if WebStorm ignores importing for node_modules that start with . something, so I renamed .prisma to myprisma and adjusted the reference in @prisma/client/index but that also didn't work.

If I change to @prisma/client/index to explicitly list the export, like so

export * from '.prisma/client'
export { GatherEvent } from '.prisma/client'

then it works fine. So my guess is that WebStorm doesn't handle export * from 'mymodule' as well as other exports.

I'd say this issue is quite reasonable to close, but also this is a big stumbling block for anyone using WebStorm. What do you think about me adding something to documentation somewhere? Or what would be the possibility of changing @prisma/client/index to just explicitly list the exports, would that be hard / have negative effects for others? Could I help with that?

@aqrln
Copy link
Member

aqrln commented Aug 10, 2022

What do you think about me adding something to documentation somewhere?

Sounds good!

Strangely, WebStorm still doesn't work. If I add .prisma as unexcluded, then it works fine.

I'm curious, does it import from @prisma/client or .prisma/client when you do that?

Or what would be the possibility of changing @prisma/client/index to just explicitly list the exports, would that be hard / have negative effects for others?

That might be problematic. I would prefer not to change any files inside the package on generation.

@AndrewSouthpaw
Copy link
Contributor Author

Hmm it seems to depend?

Inside gather-prisma-types, it gives me the choice to select the right option:

image

Inside a different monorepo package, though, it goes direct from .prisma/client, which I hadn't realized until now.

import { GatherEvent } from "gather-prisma-types/dist/node_modules/.prisma/client";

const f: GatherEvent;

Very unfortunate... My bet is that the import system can't tell @prisma/client is an option because it's being accessed via node_modules (we use lerna).

So I'm realizing my repro wasn't complete: the problem was connected to using prisma within a monorepo. Outside the repo where prisma client is defined, we can't auto-import, even from VS Code:

image

(All of the results are not from prisma, just stuff we're defining)

@AndrewSouthpaw
Copy link
Contributor Author

AndrewSouthpaw commented Aug 10, 2022

(debugging more)

Both of these don't work, just explicitly re-exporting in a file I control:

export type { GatherEvent } from "@prisma/client";
export { GatherEvent } from "@prisma/client";

I understand mutating @prisma/client/index.d.ts is a nonstarter, but FWIW doing the manual re-export from there doesn't work either, so my original suggestion doesn't even solve that problem.

Interestingly, this does work to allow me to import from a different module:

image

@AndrewSouthpaw
Copy link
Contributor Author

Just following up here, this is still an issue AFAICT, but eventually we chose to explicitly generate a different output dir outside node_modules because it was causing serious issues with TS and inferred types, something like:

The inferred type of 'SomePrismaType' cannot be named without a reference to 'path/in/node_modules'. This is likely not portable. A type annotation is necessary.

It was cropping up in multiple places, requiring explicit types that looked very ugly (especially for complicated join/select queries), and eventually we circumvented the problem by moving the types outside node_modules.

Doing so also fixed import logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/improvement An improvement to existing feature and code. team/client Issue for team Client. topic: client types Types in Prisma Client topic: dx topic: editor
Projects
None yet
Development

No branches or pull requests

3 participants