merge dev to main (v3.0.0-alpha.15) #128
Conversation
* feat: implement zmodel import * addressing PR comments * update
WalkthroughThis update introduces comprehensive support for recursive import resolution and merging in the model language loader, adds new and enhanced tests for import scenarios, and enforces stricter schema validation. Several package versions are incremented, and dependency management is refined. A GitHub Actions workflow for Discord release notifications is also added. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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
|
|
Claude finished @ymc9's task —— View job PR Review AnalysisI've reviewed this pull request that merges Review Tasks:
✅ Positive Aspects
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR merges the dev branch to main, introducing import functionality for ZModel schema files. The key changes enable models to be split across multiple files and imported into each other, supporting better code organization and reusability.
- Adds import resolution and merging functionality to handle schema files that reference other schema files
- Implements support for cyclic imports and proper symbol resolution across imported modules
- Centralizes dependency management by moving package versions to the workspace catalog
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds tmp and @types/tmp packages to the workspace catalog |
| packages/testtools/src/schema.ts | Extracts compilation logic and adds new function for in-place schema generation |
| packages/testtools/package.json | Updates tmp dependencies to use catalog versions |
| packages/runtime/test/utils.ts | Adds workDir option and improves null handling in test client creation |
| packages/runtime/test/client-api/import.test.ts | Adds comprehensive test for import functionality with cross-file model references |
| packages/language/test/import.test.ts | Adds unit tests for import merging, symbol resolution, and cyclic imports |
| packages/language/src/validators/schema-validator.ts | Improves import path handling by moving variable declaration closer to usage |
| packages/language/src/utils.ts | Simplifies import resolution by removing node_modules support and adds AST utility functions |
| packages/language/src/index.ts | Implements core import loading, merging, and validation functionality |
| packages/language/package.json | Adds tmp dependencies for testing |
| packages/cli/test/ts-schema-gen.test.ts | Adds test to verify import declaration merging in TypeScript schema generation |
| packages/cli/src/actions/action-utils.ts | Adds datasource validation and warning display for schema loading |
| packages/cli/package.json | Updates tmp dependencies to use catalog versions |
| .github/workflows/github-releases-to-discord.yml | Adds GitHub Actions workflow for Discord release notifications |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Co-authored-by: ymc9 <[email protected]>
|
Claude finished @ymc9's task —— View job PR Review AnalysisI've reviewed this pull request that merges Review Tasks:
✅ Positive Aspects
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
.github/workflows/github-releases-to-discord.yml (1)
4-14: Add a minimalpermissionsblock to harden the workflow
GitHub recommends explicitly scoping the defaultGITHUB_TOKEN. Example:jobs: github-releases-to-discord: runs-on: ubuntu-latest + permissions: + contents: readpackages/runtime/test/utils.ts (1)
118-118: Nullish coalescing usage is correct here.The past review comment expressed concern about using
??=with a potentially async function, butcreateTestProject()returns a string synchronously, so this usage is appropriate.packages/language/src/utils.ts (1)
571-579: Missing JSDoc documentation.The
findRootNodefunction needs JSDoc documentation to match the pattern established by other utility functions.packages/language/src/index.ts (2)
148-148: Use strict inequality operator.
167-167: Add JSDoc documentation for linkContentToContainer.
🧹 Nitpick comments (2)
.github/workflows/github-releases-to-discord.yml (1)
8-10: Consider upgrading toactions/checkout@v4
v4brings performance and security improvements and is now the recommended major version.- - name: Checkout - uses: actions/checkout@v3 + - name: Checkout + uses: actions/checkout@v4packages/language/src/index.ts (1)
106-116: Consider safer null handling for document access.The non-null assertion on line 113 could potentially throw if
$documentis undefined. Consider adding a null check.- langiumDocuments.deleteDocument(model.$document!.uri); - services.shared.workspace.IndexManager.remove(model.$document!.uri); + if (model.$document) { + langiumDocuments.deleteDocument(model.$document.uri); + services.shared.workspace.IndexManager.remove(model.$document.uri); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.github/workflows/github-releases-to-discord.yml(1 hunks)package.json(1 hunks)packages/cli/package.json(2 hunks)packages/cli/src/actions/action-utils.ts(3 hunks)packages/cli/test/ts-schema-gen.test.ts(2 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(2 hunks)packages/language/src/index.ts(3 hunks)packages/language/src/utils.ts(3 hunks)packages/language/src/validators/schema-validator.ts(1 hunks)packages/language/test/import.test.ts(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/test/client-api/import.test.ts(1 hunks)packages/runtime/test/utils.ts(5 hunks)packages/sdk/package.json(1 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(3 hunks)packages/testtools/src/schema.ts(2 hunks)packages/typescript-config/package.json(1 hunks)packages/zod/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/cli/src/actions/action-utils.ts (2)
packages/language/src/generated/ast.ts (1)
isDataSource(417-419)packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/cli/test/ts-schema-gen.test.ts (2)
packages/testtools/src/project.ts (1)
createTestProject(5-67)packages/testtools/src/schema.ts (1)
generateTsSchemaInPlace(76-83)
packages/language/src/utils.ts (1)
packages/language/src/ast.ts (2)
AstNode(4-4)AstNode(66-71)
packages/testtools/src/schema.ts (1)
packages/sdk/src/ts-schema-generator.ts (1)
TsSchemaGenerator(55-1414)
packages/language/src/index.ts (4)
packages/language/src/utils.ts (5)
resolveImport(395-413)getDocument(562-569)resolveTransitiveImports(364-366)getDataModelAndTypeDefs(428-435)hasAttribute(52-54)packages/language/src/ast.ts (2)
AstNode(4-4)AstNode(66-71)packages/language/src/generated/ast.ts (1)
isDataSource(417-419)packages/sdk/src/model-utils.ts (1)
hasAttribute(58-63)
🪛 actionlint (1.7.7)
.github/workflows/github-releases-to-discord.yml
9-9: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Check: CodeQL
.github/workflows/github-releases-to-discord.yml
[warning] 6-14: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (42)
samples/blog/package.json (1)
3-3: Version bump looks consistentThe update to
3.0.0-alpha.15matches the monorepo release train. No further action needed.packages/zod/package.json (1)
3-3: Aligned with workspace version
@zenstackhq/zodnow advertises3.0.0-alpha.15, keeping peer packages in sync.packages/sdk/package.json (1)
3-3: SDK version bumped correctlyChange is purely declarative; build/publish scripts will pick up the new tag.
packages/common-helpers/package.json (1)
3-3: Helper package version updatedNothing else changed—good for release consistency.
package.json (1)
3-3: Root version updatedTop-level
package.jsonnow matches all workspace packages. Ensure lock-file regeneration before publishing.Please run
pnpm installto refresh the lockfile and confirm no mismatched versions remain.packages/create-zenstack/package.json (1)
3-3: Version bump to 3.0.0-alpha.15 looks goodNo issues spotted; this keeps the package in sync with the coordinated release across the monorepo.
packages/typescript-config/package.json (1)
3-3: Version updated correctlyThe new version matches the rest of the workspace packages; nothing else to address.
packages/eslint-config/package.json (1)
3-3: ESLint config version alignedChange is minimal and consistent with the release tag.
packages/runtime/package.json (1)
3-3: Runtime package version increment verifiedBump is coherent with other workspace packages; no additional adjustments required.
tests/e2e/package.json (1)
3-3: E2E tests package version updated appropriatelyVersion aligns with the monorepo release set. Good to go.
packages/ide/vscode/package.json (1)
4-4: Version bump aligns with the coordinated release – LGTM
No functional or metadata issues observed.packages/tanstack-query/package.json (1)
3-3: Package version updated consistently – LGTM
Nothing else changed; looks good.pnpm-workspace.yaml (1)
14-15: tmp usage confined to tests and CLI only
I ran the suggested grep and foundimport tmp from 'tmp'only in:
- packages/testtools/src/project.ts
- packages/cli/test/init.test.ts
- packages/language/test/import.test.ts
Since
tmp@^0.2.3is only pulled in by non-production code (tests/CLI), it’s safe to include as a dev/test dependency. Ensure your CVE-monitoring process covers this package, and this is good to merge.packages/cli/package.json (1)
46-54: Ensuretmpis not required at runtime
tmpand its types were moved to"catalog:"underdevDependencies. If any CLI code (outside test files) invokestmp, the package will be missing when users install the CLI globally. Double-check that all imports live strictly in test helpers.packages/language/package.json (2)
4-4: Version update looks good.The version bump to 3.0.0-alpha.15 aligns with the PR objectives for this release.
66-67: New dependencies appropriately added.The addition of
tmpand@types/tmpdependencies using the catalog pattern supports the new import testing functionality and aligns with similar additions across other packages.packages/language/src/validators/schema-validator.ts (1)
51-51: Good refactoring: defer variable initialization.Moving the
importPathvariable declaration inside the conditional block is a good practice that defers computation until needed and keeps the variable scope tighter.packages/testtools/package.json (2)
3-3: Version update is consistent.The version bump to 3.0.0-alpha.15 is consistent with the coordinated release across packages.
34-34: Dependency management improvements.Switching from fixed versions to the catalog pattern for
tmpand@types/tmpimproves dependency management consistency across the monorepo.Also applies to: 44-44
packages/cli/test/ts-schema-gen.test.ts (2)
2-4: New imports support enhanced testing capabilities.The additional imports for
createTestProject,generateTsSchemaInPlace,fs, andpathproperly support the new test case for import merging functionality.
331-362: Excellent test coverage for import merging.This test case thoroughly validates the import merging functionality by:
- Creating isolated test environment with temporary project
- Testing cross-file import resolution (enum from a.zmodel used in b.zmodel)
- Verifying that both imported and local declarations are properly merged in the generated schema
The test structure follows good practices and provides valuable coverage for the new import handling capabilities.
packages/cli/src/actions/action-utils.ts (3)
2-2: Appropriate import for enhanced validation.The import of
isDataSourcefrom the language package supports the new datasource validation logic.
45-47: Improved warning visibility.Adding yellow color to warning messages improves user experience by making warnings more distinguishable from errors.
61-63: Enhanced schema validation with datasource requirement.The new validation ensuring at least one datasource is present strengthens schema correctness, as Prisma schemas require a datasource to be functional. This aligns with the broader validation improvements in this PR.
packages/language/test/import.test.ts (4)
9-34: Well-structured test for declaration merging.The test correctly verifies that importing declarations from another file merges them into the main model and removes the import statements from the final AST structure.
36-60: Excellent test for symbol resolution across imports.This test validates that imported symbols (Role enum) are properly resolved and referenced in the importing model, which is crucial for type checking and validation.
62-90: Great coverage for cyclic import scenarios.Testing bidirectional imports between models A and B ensures the import resolution handles circular dependencies correctly, which is a common real-world scenario.
92-100: Clean helper function with proper error handling.The
expectLoadedhelper provides good error reporting and validation, making test failures easier to debug.packages/runtime/test/client-api/import.test.ts (2)
7-46: Excellent integration test setup.The test creates a realistic scenario with mutual imports between User and Post models, properly demonstrating relational modeling across imported files. The setup with separate schema files and a main schema is a common real-world pattern.
48-70: Comprehensive end-to-end validation.The test validates the entire pipeline from import resolution through schema generation to actual database operations. The nested create operation with inclusion properly tests that relations work correctly across imported models.
packages/runtime/test/utils.ts (2)
65-65: Good addition of workDir option.Adding the
workDiroption toCreateTestClientOptionsprovides flexibility for tests to specify their own working directory, which is essential for the new import testing scenarios.
135-137: Conditional logging improves debugging experience.Only logging the work directory when it's available prevents confusing output and helps with debugging test scenarios.
packages/language/src/utils.ts (3)
2-2: Good addition of required Langium types.Adding
AstNode,LangiumDocument, etc. to the imports supports the new utility functions for document and AST node manipulation.
416-423: Simplified import resolution is more maintainable.The simplified
resolveImportUrifunction removes complex node_modules resolution logic in favor of straightforward relative path resolution, which is more predictable and easier to maintain.
556-569: Useful document retrieval utility.The
getDocumentfunction provides a clean way to retrieve the LangiumDocument from any AST node with proper error handling when the document is not found.packages/testtools/src/schema.ts (3)
57-58: Good refactoring to extract common logic.Moving the compilation and loading logic to a separate
compileAndLoadfunction improves code organization and reusability.
60-69: Clean helper function for compilation and loading.The
compileAndLoadfunction centralizes the TypeScript compilation and module loading logic, making it reusable across different schema generation scenarios.
76-83: Useful addition for in-place schema generation.The
generateTsSchemaInPlacefunction enables schema generation directly in an existing directory, which is essential for testing import scenarios where multiple schema files need to coexist.packages/language/src/index.ts (4)
1-1: Import statements look good.The added imports are appropriate for the new import resolution and merging functionality.
Also applies to: 6-6, 9-9
64-69: Import loading implementation looks good.The recursive loading of imported documents is correctly implemented.
152-165: Import merging logic is well implemented.The function correctly handles transitive import resolution, declaration merging, and container relationship fixing.
186-200: Validation logic looks good.The function correctly enforces the constraints for datasource and @@auth declarations after import merging.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores