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

chore (client): pin Zod to 3.21.1 to avoid code generator bug in client #700

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/happy-lobsters-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"electric-sql": patch
"@core/electric": patch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changeset is not correct. It patches all packages but your PR only touches the typescript client (i.e. electric-sql) and the generator (i.e. @electric-sql/prisma-generator). It does not touch Electric (@core/electric) neither does it touch the starter ("create-electric-app": patch).

So the changeset should look like this:

---
electric-sql": patch
@electric-sql/prisma-generator": patch
---

Pin Zod to version 3.21.1

You probably accidentally selected all packages instead of only the changed packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed it only listed the ones it detected changes in so I accepted the defaults.

I’ll push an updated one tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, since you changed the ts-client and the generator it should ask something like:

Which packages would you like to include? … 
◯ changed packages
  ◯ electric-sql
  ◯ @electric-sql/prisma-generator
◯ unchanged packages
  ◯ @core/electric
  ◯ create-electric-app

And you should only check "changed packages" (by pressing the spacebar).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevin-dp : It didn't show it like that for me, but perhaps that is me not branching in some expected way.
image

I have now re-generated the changeset as per your instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevin-dp, any feedback? I assume I shouldn’t resolve this thread?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fooware, sorry for the delay. We are discussing this internally to see whether we want to pin Zod to a specific version (which affects all user projects) or if we can provide a less invasive fix. Will keep you posted today hopefully!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevin-dp, any feedback? I assume I shouldn’t resolve this thread?

We will need a bit more time to see if we can provide a fix for this under the hood such that we don't need to pin the version of Zod to a specific version for all Electric projects. This will require some changes in the generator. I'm hoping to look into it tomorrow and ideally push a fix for it tomorrow. We will keep you posted here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zod issue is open for some time now and has still not been confirmed, so I think there will not be a fix any time soon. I circumvented the issue by adding an option to use type assertions in zod-prisma-types. It is a brute force method so I can personally use it with latest zod versions and it bypasses all the nice type inference, but since the library is about runtime checks - and these seem to work - it is an acceptable risk I'm willing to take imost of the time

I don't know if this is a valid option in your case but since the types worked pre 3.21.2 there seems to be nothing wrong with the prisma types and with asserting them.

"create-electric-app": patch
"@electric-sql/prisma-generator": patch
---

Pin Zod to version 3.21.1
2 changes: 1 addition & 1 deletion clients/typescript/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
"uuid": "^9.0.0",
"walkjs": "^3.2.4",
"ws": "^8.8.1",
"zod": "^3.20.2"
"zod": "3.21.1"
},
"devDependencies": {
"@ikscodes/browser-env": "^1.0.0",
Expand Down
2 changes: 1 addition & 1 deletion e2e/satellite_client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"electric-sql": "workspace:*",
"jsonwebtoken": "^9.0.0",
"uuid": "^9.0.0",
"zod": "^3.21.4"
"zod": "3.21.1"
},
"devDependencies": {
"@electric-sql/prisma-generator": "workspace:*",
Expand Down
2 changes: 1 addition & 1 deletion examples/linearlite/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"uuid": "^9.0.0",
"vite-plugin-svgr": "^3.2.0",
"wa-sqlite": "rhashimoto/wa-sqlite#semver:^0.9.8",
"zod": "^3.22.2"
"zod": "3.21.1"
},
"devDependencies": {
"@databases/pg": "^5.4.1",
Expand Down
2 changes: 1 addition & 1 deletion generator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@
"@prisma/generator-helper": "^4.11.0",
"code-block-writer": "^11.0.3",
"lodash": "^4.17.21",
"zod": "^3.21.1"
"zod": "3.21.1"
}
}
16 changes: 8 additions & 8 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading