-
-
Notifications
You must be signed in to change notification settings - Fork 11
merge dev to main #72
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
Conversation
* feat: implement raw queries * add coderabbit config * fixes
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
WalkthroughThis update introduces raw SQL query support to the client API, adds comprehensive tests for these features, and updates version numbers across multiple packages to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClientImpl
participant QueryExecutor
participant Kysely
User->>ClientImpl: $executeRaw / $queryRaw (template or unsafe)
ClientImpl->>QueryExecutor: executeQuery(compiledQuery, parameters)
QueryExecutor->>Kysely: proceedQueryWithKyselyInterceptors(queryNode, parameters)
Kysely-->>QueryExecutor: Query result / affected rows
QueryExecutor-->>ClientImpl: Result
ClientImpl-->>User: Promise<Result>
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/runtime/src/client/contract.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/runtime/src/client/client-impl.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by packages/runtime/src/client/executor/zenstack-query-executor.tsOops! Something went wrong! :( ESLint: 9.29.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Merge branch dev into main, bumping package versions to 3.0.0-alpha.8, introducing raw SQL query support, and updating version bump tooling.
- Bumped version numbers across all packages and samples from
3.0.0-alpha.7to3.0.0-alpha.8. - Added
$executeRaw,$executeRawUnsafe,$queryRaw, and$queryRawUnsafemethods in the client implementation and defined corresponding types in the contract, along with end-to-end tests. - Adjusted the
bump-versionworkflow to usenpx tsxand added an npm badge toREADME.md; configuredturbo.jsoninputs for incremental builds.
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Added inputs to the build task |
| package.json files across multiple packages | Bumped versions |
| packages/testtools/src/schema.ts | Tweaked log message |
| packages/runtime/src/client/contract.ts | Declared raw query methods in client contract |
| packages/runtime/src/client/client-impl.ts | Implemented raw query methods and query compiler |
| packages/runtime/src/client/executor/zenstack-query-executor.ts | Passed raw query parameters through interceptors |
| packages/runtime/test/client-api/raw-query.test.ts | New tests for raw query methods |
| packages/runtime/test/client-api/client-specs.ts | Changed createClientSpecs signature |
| package.json | Updated scripts for merge-main and bump-version |
| .github/workflows/bump-version.yml | Switched bump step to npx tsx scripts/bump-version.ts |
| README.md | Added npm version badge |
| .coderabbit.yaml | Added auto-review configuration |
Comments suppressed due to low confidence (3)
packages/runtime/src/client/client-impl.ts:216
- The function
createDeferredPromiseis used here but not imported; addimport { createDeferredPromise } from '@zenstackhq/common-helpers';(or the correct module) to avoid a runtime error.
return createDeferredPromise(async () => {
packages/runtime/test/client-api/client-specs.ts:6
- The
providersparameter is typed asstring[], which allows any string; consider using a union type likeArray<'sqlite'|'postgresql'>to retain compile-time checks.
export function createClientSpecs(dbName: string, logQueries = false, providers: string[] = ['sqlite', 'postgresql']) {
packages/testtools/src/schema.ts:38
- [nitpick] The log message uses
Work directory:which is less common thanWorking directory:; consider reverting for clarity and consistency.
console.log(`Work directory: ${workDir}`);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/testtools/src/schema.ts (1)
38-38: Minor wording regression – consider reverting to “Working directory”“Working directory” is the commonplace phrasing in build / CLI output; “Work directory” reads a bit off-beat and may confuse grep-based tooling looking for the old token.
Unless there’s a deliberate reason, consider restoring the original copy.README.md (1)
9-11: Fix missing alt text for accessibility compliance.The npm badge image is missing alt text, which impacts accessibility for screen readers.
Apply this diff to add appropriate alt text:
- <a href="https://www.npmjs.com/package/@zenstackhq/cli?activeTab=versions"> - <img src="https://img.shields.io/npm/v/%40zenstackhq%2Fcli/next"> - </a> + <a href="https://www.npmjs.com/package/@zenstackhq/cli?activeTab=versions"> + <img src="https://img.shields.io/npm/v/%40zenstackhq%2Fcli/next" alt="npm version"> + </a>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.coderabbit.yaml(1 hunks).github/workflows/bump-version.yml(1 hunks)README.md(1 hunks)package.json(2 hunks)packages/cli/package.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/client-impl.ts(2 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/executor/zenstack-query-executor.ts(3 hunks)packages/runtime/test/client-api/client-specs.ts(1 hunks)packages/runtime/test/client-api/raw-query.test.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/testtools/src/schema.ts(1 hunks)packages/typescript-config/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/runtime/test/client-api/raw-query.test.ts (2)
packages/runtime/test/client-api/client-specs.ts (1)
createClientSpecs(6-42)packages/runtime/src/client/contract.ts (1)
ClientContract(35-133)
🪛 markdownlint-cli2 (0.17.2)
README.md
10-10: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (33)
tests/e2e/package.json (1)
3-3: Version bump aligns with the monorepo release cadence – looks good.packages/common-helpers/package.json (1)
3-3: Consistent version update; no issues detected.packages/typescript-config/package.json (1)
3-3: Version increment is correct and consistent.packages/sdk/package.json (1)
3-3: Version bump matches other workspace packages – looks good.samples/blog/package.json (1)
3-3: Sample project version updated correctly.packages/tanstack-query/package.json (1)
3-3: Version bump LGTM
3.0.0-alpha.8is consistent with the monorepo update and nothing else changed.packages/cli/package.json (1)
6-6: Version bump LGTM
CLI package follows the same3.0.0-alpha.8bump as the rest of the workspace.packages/eslint-config/package.json (1)
3-3: Version bump LGTM
No other metadata touched; keeps workspace versions in sync.packages/create-zenstack/package.json (1)
3-3: Version bump LGTM
Aligned with other package versions, no additional changes.packages/zod/package.json (1)
3-3: Version bump LGTM
@zenstackhq/zodcorrectly updated to3.0.0-alpha.8; dependencies remain untouched.packages/testtools/package.json (1)
3-3: Confirm ancillary release artefacts reflect 3.0.0-alpha.8Version bump looks good.
Please verify that:
• CHANGELOG / release notes include an entry for α.8
• Any publish / tag automation scripts pick up the new version.packages/ide/vscode/package.json (1)
4-4: VS Code marketplace alignment checkMarketplace requires each prerelease version to be unique. Ensure the extension wasn’t already published with this exact semver; otherwise
vsce publishwill fail..coderabbit.yaml (1)
1-8: Config validates against v2 schema – LGTMYAML matches the provided schema: language enum, early_access flag, nested
reviews.auto_review.enabled, andchat.auto_replyare all valid. No action needed.packages/language/package.json (1)
4-4: Package version bump acknowledgedNo other changes detected; version consistency maintained across workspace packages.
packages/runtime/package.json (1)
3-3: LGTM! Version bump looks correct.The alpha version increment follows semantic versioning conventions and aligns with the coordinated release across the monorepo.
turbo.json (1)
6-6: Good addition for build optimization.Adding the
inputsfield helps Turbo understand what files should trigger a rebuild, improving build caching efficiency.packages/runtime/test/client-api/client-specs.ts (1)
6-6: Good improvement to function flexibility.Changing from a tuple literal type to a string array with default value makes the function more flexible while maintaining backward compatibility. This supports testing with different database provider combinations.
.github/workflows/bump-version.yml (1)
37-37: Good refactoring to direct script execution.Changing from
pnpm run bump-versionto directnpx tsx scripts/bump-version.tsexecution removes unnecessary indirection and makes the workflow more straightforward.package.json (2)
3-3: LGTM: Version bump is consistent with alpha release cycle.The version increment from
3.0.0-alpha.7to3.0.0-alpha.8follows semantic versioning for alpha releases.
13-14: LGTM: Workflow automation improvements.The addition of the
merge-mainscript and the change to use GitHub Actions for version bumping improves automation and consistency across environments.packages/runtime/src/client/executor/zenstack-query-executor.ts (3)
84-86: LGTM: Correct raw query parameter detection and handling.The detection of raw queries using the
$rawflag and parameter extraction is implemented correctly. The type assertion is necessary since$rawis a custom property added toCompiledQueryfor raw query identification.
101-106: LGTM: Method signature updates properly thread parameters.The addition of the
parametersargument to both interceptor and execution methods ensures raw query parameters are preserved throughout the execution pipeline.
137-141: LGTM: Parameter merging preserves raw query parameters.The conditional parameter merging ensures that raw query parameters override any compiled parameters, which is the correct behavior for raw queries.
packages/runtime/test/client-api/raw-query.test.ts (4)
8-8: LGTM: Excellent test parameterization for multi-provider support.Using
createClientSpecsto test across multiple database providers ensures the raw query functionality works consistently across different databases.
19-31: LGTM: Comprehensive test coverage for safe raw execution.The test correctly verifies the
$executeRawmethod using tagged template literals, checks the affected row count, and validates the data was actually updated.
41-47: LGTM: Proper handling of provider-specific SQL syntax.The conditional SQL generation based on provider type correctly handles parameter placeholder differences between PostgreSQL (
$1,$2) and SQLite (?) syntax.
57-62: LGTM: Complete test coverage for raw query operations.The tests for
$queryRawand$queryRawUnsafeprovide comprehensive coverage, testing both safe tagged templates and unsafe string queries with proper type assertions.packages/runtime/src/client/contract.ts (2)
43-61: LGTM: Well-designed API with clear security distinctions.The interface properly distinguishes between safe (prepared) and unsafe (direct string) variants. The security warnings in the documentation for unsafe methods are essential and well-placed.
62-80: LGTM: Consistent API design with proper typing.The query methods follow the same safe/unsafe pattern as the execute methods, with appropriate generic typing for flexible result handling. The documentation examples are clear and helpful.
packages/runtime/src/client/client-impl.ts (4)
4-4: LGTM: Appropriate imports for raw query functionality.The addition of
CompiledQueryandsqlimports from Kysely provides the necessary building blocks for both safe and unsafe raw query implementations.Also applies to: 10-10
215-220: LGTM: Safe raw execution using Kysely's tagged template.The implementation correctly uses Kysely's
sqltagged template literal which provides built-in protection against SQL injection. The numeric conversion ofnumAffectedRowsensures consistent return types.
222-228: LGTM: Unsafe raw execution with proper query compilation.The unsafe method correctly uses the helper to create a compiled raw query. The implementation maintains consistency with the safe variant while allowing direct SQL execution.
245-248: LGTM: Correct raw query compilation with identification flag.The helper method properly creates a
CompiledQueryfrom raw SQL and adds the$rawflag that the executor uses to identify and handle raw queries correctly.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores