Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 WalkthroughDocumentation updates across the ZenStack site: MySQL support added to database compatibility notes and examples, MySQL-specific code snippets and tabs introduced, feature compatibility clarifications updated, a new CLI proxy command documented, and a community package entry for zenstack-encrypt added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/reference/cli.md (1)
22-31:⚠️ Potential issue | 🟡 MinorAdd
proxyto the top-level Commands list.All other commands (
generate,migrate,db, etc.) are listed in the top-level usage block for discoverability.proxyis absent.✏️ Suggested fix
Commands: generate [options] Run code generation. migrate Run database schema migration related tasks. db Manage your database schema during development. info [path] Get information of installed ZenStack packages. init [path] Initialize an existing project for ZenStack. check [options] Check a ZModel schema for syntax or semantic errors. format [options] Format a ZModel schema file. + proxy [options] Start the ZenStack proxy server (alias: studio). help [command] display help for command🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/cli.md` around lines 22 - 31, The top-level "Commands:" usage block is missing the proxy command; add an entry "proxy [options] Run the proxy server or related proxy tasks." to the Commands list so it appears alongside generate, migrate, db, info, init, check, format and help for discoverability; update the same block that currently lists generate/migrate/db/etc. and ensure the formatting aligns with the existing entries.docs/orm/client.md (1)
19-19:⚠️ Potential issue | 🟡 MinorUpdate the description to include MySQL.
The text still only mentions SQLite and PostgreSQL; the MySQL tab added below is not reflected.
✏️ Suggested fix
-The samples below show creating a client using SQLite (via [better-sqlite3](https://github.com/WiseLibs/better-sqlite3)) and PostgreSQL (via [node-postgres](https://github.com/brianc/node-postgres)). You can also use any other Kysely dialects for these two types of databases. +The samples below show creating a client using SQLite (via [better-sqlite3](https://github.com/WiseLibs/better-sqlite3)), PostgreSQL (via [node-postgres](https://github.com/brianc/node-postgres)), and MySQL (via [mysql2](https://github.com/sidorares/node-mysql2)). You can also use any other Kysely dialects for these database types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/orm/client.md` at line 19, Update the opening sentence that currently reads "The samples below show creating a client using SQLite (via better-sqlite3) and PostgreSQL (via node-postgres)." to also mention MySQL—e.g., "The samples below show creating a client using SQLite (via better-sqlite3), PostgreSQL (via node‑postgres), and MySQL (via the appropriate Kysely MySQL dialect)." Ensure the sentence still notes that any other Kysely dialects for these database types can be used so the new MySQL tab is reflected.
🧹 Nitpick comments (2)
docs/community-packages.md (1)
10-13: Consider placing the entry in alphabetical order with the rest of the list.The existing entries appear in alphabetical order (
zenstack-cache,zenstack-kit,zenstack-live,zenstack-pinia-colada,zenstack-trpc). Insertingzenstack-encryptat the top breaks that order; it would fit naturally betweenzenstack-cacheandzenstack-kit.✏️ Suggested reordering
- -- [zenstack-encrypt](https://github.com/genu/zenstack-encryption) - - Transparent field-level encryption and decryption using the `@encrypted` attribute. Made by [`@genu`](https://github.com/genu). - - [zenstack-cache](https://github.com/visualbravo/zenstack-cache) Reduce response times and database load with query-level caching integrated with the ZenStack ORM. Made by [`@sanny-io`](https://github.com/sanny-io). +- [zenstack-encrypt](https://github.com/genu/zenstack-encryption) + + Transparent field-level encryption and decryption using the `@encrypted` attribute. Made by [`@genu`](https://github.com/genu). + - [zenstack-kit](https://github.com/visualbravo/zenstack-kit)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/community-packages.md` around lines 10 - 13, The list entry for "zenstack-encrypt" is out of alphabetical order; move the markdown block for zenstack-encrypt so it appears between the existing "zenstack-cache" and "zenstack-kit" entries (i.e., place the `zenstack-encrypt` list item after `zenstack-cache` and before `zenstack-kit`) to restore alphabetical ordering in docs/community-packages.md.docs/modeling/datasource.md (1)
16-38: Consider adding a MySQL tab to match the updated support statement.Line 38 now advertises MySQL as a supported database, but the
<Tabs>block above it only shows PostgreSQL and SQLite examples.docs/orm/client.mdadds a MySQL tab in the equivalent client setup section — a matching MySQL datasource example here would keep the docs consistent.✏️ Suggested addition
</TabItem> + +<TabItem value="mysql" label="MySQL"> +```zmodel +datasource db { + provider = 'mysql' + url = env('DATABASE_URL') +} +``` +</TabItem> </Tabs>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/modeling/datasource.md` around lines 16 - 38, Add a third TabItem for MySQL inside the existing <Tabs> block to match the supported DB list: create a <TabItem value="mysql" label="MySQL"> containing a zmodel datasource block for datasource db with provider = 'mysql' and url = env('DATABASE_URL'), and place it alongside the existing <TabItem> entries (between the SQLite entry and the closing </Tabs>) so the docs show PostgreSQL, SQLite and MySQL examples consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/faq.md`:
- Line 13: Fix the missing space between sentences in the FAQ text: locate the
sentence fragment "SQLite (with [better-sqlite3]... and MySQL (with [mysql2]...)
are supported.There's no plan to support other relational databases or NoSQL
databases." and insert a space after the period so it reads "...are supported.
There's no plan to support other relational databases or NoSQL databases." in
docs/faq.md.
In `@docs/migrate-prisma.md`:
- Around line 108-123: The MysqlDialect import is wrong — update the import in
the db.ts snippet to import { MysqlDialect } from 'kysely' instead of
'@zenstackhq/orm/dialects/mysql', keep the ZenStackClient and schema usage, and
adjust createPool to pass the connection as an object (createPool({ uri:
process.env.DATABASE_URL })) as shown in the review; also ensure the consumer
installs the runtime packages (kysely and mysql2) before running.
In `@docs/orm/client.md`:
- Line 73: The snippet passes process.env.DATABASE_URL (type string | undefined)
directly to mysql2.createPool which rejects undefined; update the code around
createPool so it guarantees a string before calling createPool — e.g., check or
assert that process.env.DATABASE_URL is defined (throw a clear error if missing)
and then pass that validated string to createPool, or provide a safe default
connection string; reference the process.env.DATABASE_URL symbol and the
createPool call when making this change.
In `@docs/reference/cli.md`:
- Around line 294-303: The proxy command's bash code block is missing the
standard "Usage:" header and brief description, breaking consistency; update the
proxy block (the bash fenced code that currently starts with "Options:") to
include a leading "Usage: zen proxy [options]" line and a short description like
"Start the ZenStack proxy server." immediately above the existing "Options:"
section so it matches the other command blocks' format.
In `@docs/reference/zmodel/datasource.md`:
- Line 32: Update the inline URL example in the datasource description to
include a MySQL example alongside the PostgreSQL one: keep the PostgreSQL
example (postgresql://user:password@localhost:5432/dbname) and add a MySQL
example (e.g., mysql://user:password@localhost:3306/dbname) so readers
configuring MySQL have a concrete format to follow; edit the sentence in
docs/reference/zmodel/datasource.md to present both examples.
---
Outside diff comments:
In `@docs/orm/client.md`:
- Line 19: Update the opening sentence that currently reads "The samples below
show creating a client using SQLite (via better-sqlite3) and PostgreSQL (via
node-postgres)." to also mention MySQL—e.g., "The samples below show creating a
client using SQLite (via better-sqlite3), PostgreSQL (via node‑postgres), and
MySQL (via the appropriate Kysely MySQL dialect)." Ensure the sentence still
notes that any other Kysely dialects for these database types can be used so the
new MySQL tab is reflected.
In `@docs/reference/cli.md`:
- Around line 22-31: The top-level "Commands:" usage block is missing the proxy
command; add an entry "proxy [options] Run the proxy server or related proxy
tasks." to the Commands list so it appears alongside generate, migrate, db,
info, init, check, format and help for discoverability; update the same block
that currently lists generate/migrate/db/etc. and ensure the formatting aligns
with the existing entries.
---
Nitpick comments:
In `@docs/community-packages.md`:
- Around line 10-13: The list entry for "zenstack-encrypt" is out of
alphabetical order; move the markdown block for zenstack-encrypt so it appears
between the existing "zenstack-cache" and "zenstack-kit" entries (i.e., place
the `zenstack-encrypt` list item after `zenstack-cache` and before
`zenstack-kit`) to restore alphabetical ordering in docs/community-packages.md.
In `@docs/modeling/datasource.md`:
- Around line 16-38: Add a third TabItem for MySQL inside the existing <Tabs>
block to match the supported DB list: create a <TabItem value="mysql"
label="MySQL"> containing a zmodel datasource block for datasource db with
provider = 'mysql' and url = env('DATABASE_URL'), and place it alongside the
existing <TabItem> entries (between the SQLite entry and the closing </Tabs>) so
the docs show PostgreSQL, SQLite and MySQL examples consistently.
|
|
||
| export const db = new ZenStackClient(schema, { | ||
| dialect: new MysqlDialect({ | ||
| pool: createPool(process.env.DATABASE_URL), |
Contributor
There was a problem hiding this comment.
process.env.DATABASE_URL is string | undefined — TypeScript will reject this.
mysql2's createPool signature is createPool(config: string | PoolOptions): Pool — it does not accept undefined. Unlike the PostgreSQL example where connectionString accepts string | undefined, passing process.env.DATABASE_URL directly here will produce a compile error for anyone copying this snippet.
✏️ Suggested fix
- pool: createPool(process.env.DATABASE_URL),
+ pool: createPool(process.env.DATABASE_URL!),or more defensively:
- pool: createPool(process.env.DATABASE_URL),
+ pool: createPool(process.env.DATABASE_URL as string),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pool: createPool(process.env.DATABASE_URL), | |
| pool: createPool(process.env.DATABASE_URL!), |
Suggested change
| pool: createPool(process.env.DATABASE_URL), | |
| pool: createPool(process.env.DATABASE_URL as string), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/orm/client.md` at line 73, The snippet passes process.env.DATABASE_URL
(type string | undefined) directly to mysql2.createPool which rejects undefined;
update the code around createPool so it guarantees a string before calling
createPool — e.g., check or assert that process.env.DATABASE_URL is defined
(throw a clear error if missing) and then pass that validated string to
createPool, or provide a safe default connection string; reference the
process.env.DATABASE_URL symbol and the createPool call when making this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit