-
Notifications
You must be signed in to change notification settings - Fork 174
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
Generated client in 0.9.x can still contain "string is not assignable to type undefined" errors #1001
Comments
Hi @fooware, i found that these type errors got introduced by our upgrade to Prisma v5 for generating the Prisma client as v5 seems to introduce some breaking changes in the generated types. I could reproduce your type errors on a fresh |
Created a PR for reverting to Prisma v4: #1016 |
Sounds great @kevin-dp, looking forward to testing the new release when it is out. |
Could this be related to this long lived Zod bug? colinhacks/zod#2184 |
@fooware this issue should be resolved now in 0.9.6 - can you confirm? |
@msfstef I am still seeing errors in 0.9.6 and had to add |
@jljorgenson18 is it still the |
I am receiving two types of errors. The first one is the string error
The second one has something to do with context
|
Thanks for the update @msfstef . I'm heading out for easter vacation at this very moment, but I will try to update the test case repo and check during next week. |
@jljorgenson18 could you post your tsconfig compiler options here? Make sure you have |
@msfstef Everything is set to strict, otherwise the undefined error would not be possible I don't think. {
"compilerOptions": {
"target": "esnext",
"moduleResolution": "node",
"module": "ESNext",
"allowJs": true,
"strict": true,
"noErrorTruncation": true,
"esModuleInterop": true,
"noImplicitAny": true,
"resolveJsonModule": true,
"skipLibCheck": true,
"strictNullChecks": true,
"jsx": "react",
"lib": ["DOM", "ES7", "ES2019", "DOM.Iterable", "WebWorker"],
"baseUrl": ".",
"paths": {
"src/*": ["./src/*"],
"test/*": ["./test/*"]
},
"typeRoots": ["../node_modules/@types", "./typings"]
},
"include": [],
"exclude": []
}
|
@jljorgenson18 I see, thank you, could you share your migration as well so I can try and reproduce this particular error? |
@msfstef Sure thing
|
@jljorgenson18 thank you! I think it'll be easier if you share the postgres schema migration from which the sqlite migrations are generated if it's possible |
…ests (#1117) Relevant issues: #1001 #925 #1114 Only one thing is fixed in this PR, the `Relation` is no longer imported if the data model has no relations in order to avoid `unused imports` TS errors. Main changes are moving all of the generator tests into their own test directory rather than inside the source code, for consistency with the TS client, ease of use, and improved reusability of test utilities. I've also added a sort of integration test for the CLI generation step, which is crucial to getting a working, compileable TS client as it modifies some of the Prisma generated typings. This test generates the client given a Prisma schema and performs all appropriate modifications to it, and then attempts to compile it with TS and specified compiler options. Also found that with TS version >=5.4 we have [stricter conditional type checking](https://devblogs.microsoft.com/typescript/announcing-typescript-5-4/#notable-behavioral-changes) which breaks clients generated with Prisma versions earlier than 4.16.0 (see issue #1114), however 4.16.0 introduces breaking changes with the "extensions" feature, so I've added a hacky workaround for it: https://github.com/electric-sql/electric/pull/1117/files#diff-fb0a50df4fc86f09b81a210162984d7822276fe19c8ae6496cf5ffe7d09f5adb We can add more tests that would test various compiler options, making it easier to avoid regressions when users report them with more complicated schemas and TS compiler options. Finally, I've added a Caution banner in our Usage section in the docs about requiring TS `strict` mode, since Zod has that as a requirement as well and its the reason why we see some inconsistencies in bug reports.
@msfstef: I took some time to verify today. It took some time since 0.9.3 (or perhaps 0.9.4, not sure what was resolved) was no longer compatible with the new Docker Compose release that went live like 10 days ago, something about Anyhow, after rolling back my Docker version, upgrading to Electric That said, there sure is a lot of warnings now. In
In
|
@msfstef: Please let me know if you need anything else, or if I'm supposed to be the one to close the issue. |
@fooware glad the string issue was resolved! There's one more PR that should solve a few more TS issues (#1117). Regarding the linting, it'll be hard to make the client respect all lint rule setups so the main thing is for it to be compileable by TS - I'd suggest excluding the generated client directory from linting (@kevin-dp what do you think?) That being said, at least for the If your issue has been resolved then feel free to close it :) |
@msfstef, indeed, we don't have much (if any) control over the generated |
There have been problems with incompatible Zod and Prisma versions in previous Electric-SQL versions, requiring the developer to use specific versions of Prisma and Zod, which is somewhat complicated, see PR #700.
That PR got rejected due to #727 adding a workaround using casts in the generated client, which fixed our initial problems.
However, as late as Electric 0.9.3 we still have this problem with our generated client with a more "advanced" DB structure, and due to internal changes in the way Electric generates the client, we can no longer override the Zod version using PNPM in a good way, so we are stuck at version 0.8.1 for now.
Test case
The main branch of the test repo below contains a mostly stock
create-electric-app
using ElectricSQL 0.9.3 and it can not generate a correct client.Error
Test Repo
https://github.com/fooware/electric-testcase-01
Commit with the DB-structure
fooware/electric-testcase-01@10e5ff7
The text was updated successfully, but these errors were encountered: