fix(cli): turbo prune --docker misses tsconfig.base.json used by gene… #661
Conversation
|
@moreorover is attempting to deploy a commit to the Aman Varshney's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes relocate the base TypeScript configuration from the project root into a dedicated Changes
Sequence DiagramsequenceDiagram
participant Gen as Generator
participant TM as TemplateManager
participant WS as WorkspaceSetup
participant FS as FileSystem
Gen->>TM: setupBackendFramework(projectDir, context)
activate TM
TM->>TM: setupConfigPackage(projectDir, context)
activate TM
TM->>FS: Create packages/config
TM->>FS: Copy config templates
deactivate TM
TM->>TM: setupDbPackage(projectDir, context)
activate TM
TM->>FS: Create packages/db
TM->>FS: Copy base & ORM templates
deactivate TM
TM-->>Gen: config package ready
deactivate TM
Gen->>WS: setupWorkspaceDependencies(...)
activate WS
WS->>FS: Detect packages/config
alt config package exists
WS->>WS: Build configDev map<br/>@projectName/config@workspace:*
WS->>WS: addPackageDependency(packages/db,<br/>customDevDependencies: configDev)
WS->>WS: addPackageDependency(packages/auth,<br/>customDevDependencies: configDev)
WS->>WS: addPackageDependency(...all packages,<br/>customDevDependencies: configDev)
end
WS-->>Gen: workspace configured
deactivate WS
Note over Gen,FS: Generated tsconfig.json extends<br/>@projectName/config/tsconfig.base.json<br/>(workspace reference, included in turbo prune)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cli/test/test-utils.ts (1)
174-189: Remove explicit return types per coding guidelines.The three new utility functions have explicit return type annotations (
: string), which violates the coding guideline: "Do not use explicit return types in TypeScript."Apply this diff to align with the coding guidelines:
-export function expectSuccessWithProjectDir(result: TestResult): string { +export function expectSuccessWithProjectDir(result: TestResult) { expectSuccess(result); expect(result.projectDir).toBeDefined(); return result.projectDir as string; } /** * Helper functions for generating expected config package references */ -export function configPackageName(projectName: string): string { +export function configPackageName(projectName: string) { return `@${projectName}/config`; } -export function configTsConfigReference(projectName: string): string { +export function configTsConfigReference(projectName: string) { return `@${projectName}/config/tsconfig.base.json`; }apps/cli/test/config-package.test.ts (1)
132-169: Consider consolidating or differentiating redundant tests.Both tests in this block use
packageManager: "pnpm"and assertworkspace:*, making them functionally identical. The first test (lines 132-150) already validates pnpm workspace protocol behavior.Consider one of these approaches:
- Remove the duplicate test entirely
- Differentiate by testing other package managers (npm, yarn, bun) if they use different workspace protocols
- If the intent is to specifically document pnpm behavior, consolidate into a single test with a clearer name
For example, if you want to test multiple package managers:
it.each([ { manager: "pnpm", protocol: "workspace:*" }, { manager: "npm", protocol: "workspace:*" }, // Add others if protocols differ ])("should use correct workspace protocol for $manager", async ({ manager, protocol }) => { const projectName = `${manager}-workspace`; const result = await runTRPCTest({ projectName, backend: "hono", runtime: "node", frontend: ["tanstack-router"], packageManager: manager, install: false, }); const projectDir = expectSuccessWithProjectDir(result); const pkgJson = await readJSON(join(projectDir, "package.json")); expect(pkgJson.devDependencies[configPackageName(projectName)]).toBe(protocol); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cli/test/config-package.test.ts(1 hunks)apps/cli/test/test-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/test-utils.tsapps/cli/test/config-package.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/test-utils.tsapps/cli/test/config-package.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/test-utils.tsapps/cli/test/config-package.test.ts
🧠 Learnings (2)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/test-utils.tsapps/cli/test/config-package.test.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/config-package.test.ts
🧬 Code graph analysis (1)
apps/cli/test/config-package.test.ts (1)
apps/cli/test/test-utils.ts (5)
cleanupSmokeDirectory(31-38)runTRPCTest(57-160)expectSuccessWithProjectDir(174-178)configTsConfigReference(187-189)configPackageName(183-185)
🔇 Additional comments (1)
apps/cli/test/config-package.test.ts (1)
1-571: Excellent test coverage for the config package feature.This test suite comprehensively validates the solution to issue #620:
- Config package structure and creation
- Root tsconfig migration (no more root tsconfig.base.json)
- Workspace package references (db, api, server, auth)
- Package dependencies with workspace protocol
- Runtime-specific configuration (node/bun types)
- Edge cases (no backend, self-hosted, convex)
- Cross-stack compatibility (various backend/frontend combinations)
The test organization is clear and the coverage ensures that
turbo prune --dockerwill now correctly include the config package and its references.
|
added in some tests to aid development, I guess these tests could be brought in to the already existing test cases and generalised |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/cli/test/test-utils.ts (4)
177-181: Consider removing explicit return type.The function logic is correct, but the explicit return type
: stringcan be omitted to align with the coding guidelines.Apply this diff:
-export function expectSuccessWithProjectDir(result: TestResult): string { +export function expectSuccessWithProjectDir(result: TestResult) { expectSuccess(result); expect(result.projectDir).toBeDefined(); return result.projectDir as string; }As per coding guidelines.
186-192: Consider removing explicit return types.The string formatting logic is correct, but the explicit return types can be omitted to align with the coding guidelines.
Apply this diff:
-export function configPackageName(projectName: string): string { +export function configPackageName(projectName: string) { return `@${projectName}/config`; } -export function configTsConfigReference(projectName: string): string { +export function configTsConfigReference(projectName: string) { return `@${projectName}/config/tsconfig.base.json`; }As per coding guidelines.
201-203: Remove redundant dynamic imports.These modules are already imported at the top of the file (lines 1-5), making these dynamic imports redundant and inefficient.
Apply this diff:
- const { pathExists, readFile, readJSON } = await import("fs-extra"); - const { join } = await import("node:path"); - const { expect } = await import("vitest"); - // Extract data from result const projectDir = expectSuccessWithProjectDir(result);
198-200: Consider removing explicit return type.The explicit return type
: Promise<void>can be omitted to align with the coding guidelines.Apply this diff:
-export async function validateConfigPackageSetup( - result: TestResult, -): Promise<void> { +export async function validateConfigPackageSetup(result: TestResult) {As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cli/test/config-package.test.ts(1 hunks)apps/cli/test/test-utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cli/test/config-package.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/test-utils.ts
🧠 Learnings (2)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/test-utils.ts
🔇 Additional comments (4)
apps/cli/test/test-utils.ts (4)
45-52: LGTM!The addition of the
configfield toTestResultand the newTestConfiginterface provide a clean way to track test configuration alongside results, enabling the validation helpers to access test parameters.
147-153: LGTM!Both success and error paths correctly return the
configfield, ensuring it's always available for validation helpers.
155-162: LGTM!The conditional environment restoration correctly handles both cases: deleting the variable if it didn't exist originally, or restoring its original value.
210-320: LGTM!The validation logic comprehensively checks:
- Config package structure and files
- Package.json naming and workspace references
- TSConfig base configuration and runtime-specific types
- Root configuration updates
- Conditional workspace package validation based on the selected stack options
The conditional logic for determining which packages should exist (Lines 254-259) aligns well with the PR objectives to fix Docker pruning issues by moving configs into workspace packages.
| "build": "tsdown" | ||
| }, | ||
| "devDependencies": {}, | ||
| "devDependencies": { |
There was a problem hiding this comment.
this logic can be handled by apps/cli/src/helpers/core/workspace-setup.ts no need to add this manually
|
Thanks for the PR @moreorover 😄 |
|
all tests are kinda broken in the cli i need to find a better way to write tests 😢 If you like you can skip the tests for now (up to you) |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cli/src/helpers/core/project-config.ts(1 hunks)apps/cli/src/helpers/core/workspace-setup.ts(6 hunks)apps/cli/templates/base/package.json.hbs(1 hunks)apps/cli/test/config-package.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cli/test/config-package.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{hbs,handlebars}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
In Handlebars templates, avoid generic if/else blocks; write explicit conditions (e.g., if (eq orm "prisma") and else if (eq orm "drizzle")).
Files:
apps/cli/templates/base/package.json.hbs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/src/helpers/core/workspace-setup.tsapps/cli/src/helpers/core/project-config.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/src/helpers/core/workspace-setup.tsapps/cli/src/helpers/core/project-config.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/src/helpers/core/workspace-setup.tsapps/cli/src/helpers/core/project-config.ts
🧠 Learnings (1)
📚 Learning: 2025-08-24T18:00:39.152Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-24T18:00:39.152Z
Learning: Applies to package.json : Add types/node to package.json when using Node.js built-in modules
Applied to files:
apps/cli/src/helpers/core/project-config.ts
🧬 Code graph analysis (1)
apps/cli/src/helpers/core/workspace-setup.ts (1)
apps/cli/src/utils/add-package-deps.ts (1)
addPackageDependency(6-59)
🔇 Additional comments (3)
apps/cli/templates/base/package.json.hbs (1)
9-10: LGTM!Adding an empty
devDependenciesobject establishes the structure for the base template, allowing subsequent setup logic to populate it consistently.apps/cli/src/helpers/core/workspace-setup.ts (2)
18-23: LGTM!The config package detection logic is correct. It properly uses
workspaceVersion(which respects the package manager) and creates an empty object when the config package doesn't exist, ensuring safe propagation to alladdPackageDependencycalls.
30-30: LGTM!The config package devDependency is consistently propagated across all workspace packages (db, auth, api, server, web, native). This ensures the shared config is available during development via workspace references.
Also applies to: 46-46, 65-65, 87-87, 106-106, 123-123
moreorover
force-pushed
the
fix/config-package
branch
from
8acab10 to
6383abd
Compare
This reverts commit eef2c90.
moreorover
force-pushed
the
fix/config-package
branch
from
7660fae to
0eac7fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cli/test/config-package.test.ts (1)
1-206: Excellent test coverage! Consider making all test configurations explicit.The test suite is comprehensive and well-organized, covering basic stacks, authentication, API variants, and edge cases.
However, some tests rely on default values for critical parameters:
- Line 69: The express test doesn't specify
api- Line 80: The fastify test doesn't specify
databaseorormWhile these will fall back to defaults in
runTRPCTest, explicitly specifying all parameters would improve test clarity and maintainability.Consider applying this approach consistently:
it("should validate express + node stack", async () => { const result = await runTRPCTest({ projectName: "express-node", backend: "express", runtime: "node", database: "sqlite", orm: "drizzle", + api: "trpc", frontend: ["tanstack-router"], install: false, }); await validateConfigPackageSetup(result); }); it("should validate fastify + node stack", async () => { const result = await runTRPCTest({ projectName: "fastify-node", backend: "fastify", runtime: "node", + database: "sqlite", + orm: "drizzle", + api: "trpc", frontend: ["tanstack-router"], install: false, }); await validateConfigPackageSetup(result); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/cli/src/helpers/core/project-config.ts(1 hunks)apps/cli/src/helpers/core/template-manager.ts(2 hunks)apps/cli/src/helpers/core/workspace-setup.ts(6 hunks)apps/cli/templates/api/orpc/server/tsconfig.json.hbs(1 hunks)apps/cli/templates/api/trpc/server/tsconfig.json.hbs(1 hunks)apps/cli/templates/auth/better-auth/server/base/tsconfig.json.hbs(1 hunks)apps/cli/templates/backend/server/base/tsconfig.json.hbs(1 hunks)apps/cli/templates/base/package.json.hbs(1 hunks)apps/cli/templates/base/tsconfig.json.hbs(1 hunks)apps/cli/templates/db/base/tsconfig.json.hbs(1 hunks)apps/cli/templates/packages/config/package.json.hbs(1 hunks)apps/cli/templates/packages/config/tsconfig.json(1 hunks)apps/cli/test/config-package.test.ts(1 hunks)apps/cli/test/test-utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/cli/src/helpers/core/project-config.ts
- apps/cli/templates/db/base/tsconfig.json.hbs
- apps/cli/templates/api/trpc/server/tsconfig.json.hbs
- apps/cli/templates/packages/config/package.json.hbs
- apps/cli/src/helpers/core/template-manager.ts
- apps/cli/test/test-utils.ts
- apps/cli/templates/backend/server/base/tsconfig.json.hbs
- apps/cli/src/helpers/core/workspace-setup.ts
- apps/cli/templates/base/tsconfig.json.hbs
- apps/cli/templates/packages/config/tsconfig.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/config-package.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/config-package.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/config-package.test.ts
**/*.{hbs,handlebars}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
In Handlebars templates, avoid generic if/else blocks; write explicit conditions (e.g., if (eq orm "prisma") and else if (eq orm "drizzle")).
Files:
apps/cli/templates/base/package.json.hbsapps/cli/templates/auth/better-auth/server/base/tsconfig.json.hbsapps/cli/templates/api/orpc/server/tsconfig.json.hbs
🧠 Learnings (4)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/config-package.test.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/config-package.test.ts
📚 Learning: 2025-08-24T18:00:39.152Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-24T18:00:39.152Z
Learning: Applies to package.json : Add types/node to package.json when using Node.js built-in modules
Applied to files:
apps/cli/templates/base/package.json.hbs
📚 Learning: 2025-09-11T04:31:24.508Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/better-t-stack-repo.mdc:0-0
Timestamp: 2025-09-11T04:31:24.508Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript type aliases instead of interface declarations.
Applied to files:
apps/cli/templates/auth/better-auth/server/base/tsconfig.json.hbsapps/cli/templates/api/orpc/server/tsconfig.json.hbs
🧬 Code graph analysis (1)
apps/cli/test/config-package.test.ts (1)
apps/cli/test/test-utils.ts (3)
cleanupSmokeDirectory(31-38)runTRPCTest(58-163)validateConfigPackageSetup(198-320)
🔇 Additional comments (3)
apps/cli/templates/auth/better-auth/server/base/tsconfig.json.hbs (1)
2-2: LGTM! Extends path correctly updated to use scoped config package.The change from a relative path to the scoped
@{{projectName}}/config/tsconfig.base.jsoncorrectly implements the solution to ensure the base TypeScript configuration is included duringturbo prune --docker.apps/cli/templates/api/orpc/server/tsconfig.json.hbs (1)
2-2: LGTM! Consistent with config package migration.The extends path correctly references the scoped config package, consistent with other template updates across the codebase.
apps/cli/templates/base/package.json.hbs (1)
9-10: LGTM! Template structure prepared for config package wiring.The addition of empty
scriptsanddevDependenciesobjects provides the necessary structure for the workspace setup to inject the config package dependency (@<projectName>/config: workspace:*) whenpackages/configexists.
|
lets keep the tests in the repository I think in regards of |
|
Really need to think of a way how to test this better. I have added utility to test turborepo projects that execute |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/cli/test/test-utils.ts (1)
206-208: Type assertion could be replaced with safe fallback.The type assertion
as stringon Line 208 assumesprojectNameis always defined, butTestConfig.projectNameis optional. WhilerunTRPCTestdefaults to"default-app"(Line 70), that default is not stored back intoconfig.projectName, leaving this field potentially undefined.As discussed in previous reviews, this is a type safety concern rather than a runtime bug since all current tests explicitly provide
projectName. If preferred, you can use a safe fallback:const projectName = config.projectName ?? "default-app";
🧹 Nitpick comments (2)
apps/cli/test/config-package.test.ts (1)
17-105: Optional: Consider consistent configuration patterns across tests.Some tests explicitly specify all core stack options (e.g., lines 54-66), while others rely on defaults (e.g., lines 95-104 omit
database,orm, andapi). This works due to the default-handling logic inrunTRPCTest, but explicit configurations may improve test clarity and maintainability.apps/cli/test/test-utils.ts (1)
327-398: Consider usingBun.$instead ofexecafor shell commands.The function uses
execafor running shell commands (Lines 350, 354, 360, 387), but the coding guidelines specify usingBun.$for shelling out operations.As per coding guidelines
Example refactor for the lockfile generation section:
// Generate lockfile without installing dependencies try { if (packageManager === "pnpm") { await Bun.$`pnpm install --lockfile-only`.cwd(projectDir); } else if (packageManager === "npm") { await Bun.$`npm install --package-lock-only`.cwd(projectDir); } else if (packageManager === "bun") { await Bun.$`bun install --frozen-lockfile`.cwd(projectDir).nothrow(); } } catch (error) { // ... error handling }Similar changes would apply to the turbo prune commands (Lines 387-389).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cli/test/config-package.test.ts(1 hunks)apps/cli/test/test-utils.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/config-package.test.tsapps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/config-package.test.tsapps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/config-package.test.tsapps/cli/test/test-utils.ts
🧠 Learnings (4)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/config-package.test.tsapps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/config-package.test.tsapps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/package.json : In package.json scripts, prefer running files with `bun <file>` instead of `node <file>` or `ts-node <file>`
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/*.{js,jsx,ts,tsx,mjs,cjs} : Use `Bun.$` instead of `execa` for shelling out
Applied to files:
apps/cli/test/test-utils.ts
🧬 Code graph analysis (1)
apps/cli/test/config-package.test.ts (1)
apps/cli/test/test-utils.ts (4)
cleanupSmokeDirectory(31-38)runTRPCTest(58-163)validateConfigPackageSetup(198-320)validateTurboPrune(327-398)
🔇 Additional comments (5)
apps/cli/test/config-package.test.ts (3)
1-15: LGTM! Clean test suite setup.The imports and setup/teardown hooks are well-structured. The use of
beforeAllandafterAllto manage the.smokedirectory ensures a clean test environment.
35-52: Consider validating the runtime-specific configuration.This test uses
runtime: "workers", but the validation functionvalidateConfigPackageSetupin test-utils.ts (lines 228-232) only checks for "node" and "bun" runtime types. The "workers" runtime configuration is not validated.While this is not an issue with the test itself, you may want to extend the validation in
validateConfigPackageSetupto cover "workers" runtime as well.
156-228: Excellent edge case coverage.The edge cases section thoroughly covers important scenarios including frontend-only, convex backend, self-hosted backend, no database, and turborepo addon configurations. This comprehensive testing will help catch issues across diverse stack configurations.
apps/cli/test/test-utils.ts (2)
40-46: Good enhancement to TestResult interface.Adding the
configfield toTestResultenables validation functions to make configuration-based decisions about what to validate. This is well-utilized invalidateConfigPackageSetupto conditionally check workspace packages.
177-192: Well-designed helper functions.These helpers encapsulate common patterns effectively:
expectSuccessWithProjectDircombines success assertion with directory extractionconfigPackageNameandconfigTsConfigReferencegenerate consistent naming for validation
|
not sure how to play around wrangler configuration since it is trying to reference worker-configuration using relative path apps/cli/templates/api/orpc/server/tsconfig.json.hbs "composite": true{{#if (eq serverDeploy "wrangler")}},
"types": [
"../../apps/server/worker-configuration.d.ts"
]{{/if}}I gues it does not matter since if you are deploying using workers, pruning step is not required, although im not sure how exactly wrangler works as I have never done deployment using it @AmanVarshney01 what is your take on this? |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/cli/test/test-utils.ts (1)
206-208: Type assertion may be unsafe despite prior validation.While
expectSuccessWithProjectDirensures the test succeeded,config.projectNamecan still be undefined becauserunTRPCTestuses a local default variable (Line 70) that isn't stored back intoconfig.projectName. This creates a type safety gap.Apply this diff for better type safety:
const projectDir = expectSuccessWithProjectDir(result); const config = result.config; - const projectName = config.projectName as string; + const projectName = config.projectName ?? "default-app";
🧹 Nitpick comments (1)
apps/cli/test/test-utils.ts (1)
40-46: Consider converting interface to type alias.The coding guidelines specify using type aliases instead of interface declarations. While this interface predates the current PR, consider refactoring when convenient:
-export interface TestResult { +export type TestResult = { success: boolean; result?: InitResult; error?: string; projectDir?: string; config: TestConfig; -} +};The addition of the
configfield itself is appropriate and necessary for the validation functions.As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cli/test/test-utils.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/test-utils.ts
🧠 Learnings (4)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/package.json : In package.json scripts, prefer running files with `bun <file>` instead of `node <file>` or `ts-node <file>`
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/*.{js,jsx,ts,tsx,mjs,cjs} : Use `Bun.$` instead of `execa` for shelling out
Applied to files:
apps/cli/test/test-utils.ts
🔇 Additional comments (1)
apps/cli/test/test-utils.ts (1)
177-192: LGTM!The helper functions are well-structured and serve their purpose clearly. The type assertion on Line 180 is safe because Line 179 explicitly verifies
result.projectDiris defined.
| // Check runtime-specific types | ||
| if ( | ||
| config.runtime === "node" || | ||
| config.runtime === "workers" || | ||
| config.runtime === "none" | ||
| ) { | ||
| expect(configTsConfigBase.compilerOptions.types).toContain("node"); | ||
| } else if (config.runtime === "bun") { | ||
| expect(configTsConfigBase.compilerOptions.types).toContain("bun"); | ||
| } |
There was a problem hiding this comment.
Fix incorrect runtime validation logic.
The current logic incorrectly treats "workers" and "none" runtimes the same as "node". This conflates distinct runtime environments:
- "workers" should expect Cloudflare Workers types (e.g.,
@cloudflare/workers-types) - "none" shouldn't necessarily include "node" types
- Only "node" runtime should expect "node" types
Apply this diff to fix the validation logic:
// Check runtime-specific types
- if (
- config.runtime === "node" ||
- config.runtime === "workers" ||
- config.runtime === "none"
- ) {
+ if (config.runtime === "node") {
expect(configTsConfigBase.compilerOptions.types).toContain("node");
} else if (config.runtime === "bun") {
expect(configTsConfigBase.compilerOptions.types).toContain("bun");
+ } else if (config.runtime === "workers") {
+ // Validate Cloudflare Workers types if applicable
+ // Adjust based on your actual config template for workers runtime
+ } else if (config.runtime === "none") {
+ // Validate appropriate types for "none" runtime
+ // or skip validation if no specific types are expected
}🤖 Prompt for AI Agents
In apps/cli/test/test-utils.ts around lines 227 to 236, the runtime validation
test incorrectly groups "workers" and "none" with "node"; update the condition
so only config.runtime === "node" asserts that compilerOptions.types contains
"node", add an explicit branch for config.runtime === "workers" that asserts it
contains "@cloudflare/workers-types" (or the correct Cloudflare workers types
package), and remove or change any assertion for config.runtime === "none" so it
does not require "node" types (either assert it does not contain node or skip
type checks for "none").
| export async function validateTurboPrune(result: TestResult): Promise<void> { | ||
| const { expect } = await import("vitest"); | ||
| const { execa } = await import("execa"); | ||
|
|
||
| // Extract data from result | ||
| const projectDir = expectSuccessWithProjectDir(result); | ||
| const config = result.config; | ||
|
|
||
| // Only run this validation if turborepo addon is enabled | ||
| const hasTurborepo = | ||
| config.addons && | ||
| Array.isArray(config.addons) && | ||
| config.addons.includes("turborepo"); | ||
|
|
||
| if (!hasTurborepo) { | ||
| return; | ||
| } | ||
|
|
||
| const packageManager = config.packageManager || "pnpm"; | ||
|
|
||
| // Generate lockfile without installing dependencies | ||
| try { | ||
| if (packageManager === "pnpm") { | ||
| await execa("pnpm", ["install", "--lockfile-only"], { | ||
| cwd: projectDir, | ||
| }); | ||
| } else if (packageManager === "npm") { | ||
| await execa("npm", ["install", "--package-lock-only"], { | ||
| cwd: projectDir, | ||
| }); | ||
| } else if (packageManager === "bun") { | ||
| // Bun doesn't have --lockfile-only, so we skip for bun | ||
| // or use bun install which is fast anyway | ||
| await execa("bun", ["install", "--frozen-lockfile"], { | ||
| cwd: projectDir, | ||
| reject: false, // Don't fail if frozen-lockfile doesn't exist | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to generate lockfile:"); | ||
| console.error(error); | ||
| expect.fail( | ||
| `Failed to generate lockfile: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| } | ||
|
|
||
| // Determine package manager command for running turbo | ||
| const command = | ||
| packageManager === "npm" | ||
| ? "npx" | ||
| : packageManager === "bun" | ||
| ? "bunx" | ||
| : "pnpm"; | ||
|
|
||
| // Test turbo prune for both server and web targets | ||
| const targets = ["server", "web"]; | ||
|
|
||
| for (const target of targets) { | ||
| const args = | ||
| packageManager === "pnpm" | ||
| ? ["dlx", "turbo", "prune", target, "--docker"] | ||
| : ["turbo", "prune", target, "--docker"]; | ||
|
|
||
| try { | ||
| await execa(command, args, { | ||
| cwd: projectDir, | ||
| }); | ||
| } catch (error) { | ||
| console.error(`turbo prune ${target} failed:`); | ||
| console.error(error); | ||
| expect.fail( | ||
| `turbo prune ${target} --docker failed: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace execa with Bun.$ per coding guidelines.
The coding guidelines explicitly require using Bun.$ instead of execa for shell commands. This function uses execa on Lines 333, 354, 358, 364, and 395.
As per coding guidelines and learnings.
Apply this refactor to use Bun.$:
export async function validateTurboPrune(result: TestResult): Promise<void> {
const { expect } = await import("vitest");
- const { execa } = await import("execa");
+ const { $ } = await import("bun");
// Extract data from result
const projectDir = expectSuccessWithProjectDir(result);
const config = result.config;
// Only run this validation if turborepo addon is enabled
const hasTurborepo =
config.addons &&
Array.isArray(config.addons) &&
config.addons.includes("turborepo");
if (!hasTurborepo) {
return;
}
const packageManager = config.packageManager || "pnpm";
// Generate lockfile without installing dependencies
try {
if (packageManager === "pnpm") {
- await execa("pnpm", ["install", "--lockfile-only"], {
- cwd: projectDir,
- });
+ await $`cd ${projectDir} && pnpm install --lockfile-only`;
} else if (packageManager === "npm") {
- await execa("npm", ["install", "--package-lock-only"], {
- cwd: projectDir,
- });
+ await $`cd ${projectDir} && npm install --package-lock-only`;
} else if (packageManager === "bun") {
- await execa("bun", ["install", "--frozen-lockfile"], {
- cwd: projectDir,
- reject: false,
- });
+ await $`cd ${projectDir} && bun install --frozen-lockfile`.nothrow();
}
} catch (error) {
console.error("Failed to generate lockfile:");
console.error(error);
expect.fail(
`Failed to generate lockfile: ${error instanceof Error ? error.message : String(error)}`,
);
}
// Determine package manager command for running turbo
const command =
packageManager === "npm"
? "npx"
: packageManager === "bun"
? "bunx"
: "pnpm";
// Test turbo prune for both server and web targets
const targets = ["server", "web"];
for (const target of targets) {
const args =
packageManager === "pnpm"
? ["dlx", "turbo", "prune", target, "--docker"]
: ["turbo", "prune", target, "--docker"];
try {
- await execa(command, args, {
- cwd: projectDir,
- });
+ if (packageManager === "pnpm") {
+ await $`cd ${projectDir} && pnpm dlx turbo prune ${target} --docker`;
+ } else {
+ await $`cd ${projectDir} && ${command} turbo prune ${target} --docker`;
+ }
} catch (error) {
console.error(`turbo prune ${target} failed:`);
console.error(error);
expect.fail(
`turbo prune ${target} --docker failed: ${error instanceof Error ? error.message : String(error)}`,
);
}
}
}📝 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.
| export async function validateTurboPrune(result: TestResult): Promise<void> { | |
| const { expect } = await import("vitest"); | |
| const { execa } = await import("execa"); | |
| // Extract data from result | |
| const projectDir = expectSuccessWithProjectDir(result); | |
| const config = result.config; | |
| // Only run this validation if turborepo addon is enabled | |
| const hasTurborepo = | |
| config.addons && | |
| Array.isArray(config.addons) && | |
| config.addons.includes("turborepo"); | |
| if (!hasTurborepo) { | |
| return; | |
| } | |
| const packageManager = config.packageManager || "pnpm"; | |
| // Generate lockfile without installing dependencies | |
| try { | |
| if (packageManager === "pnpm") { | |
| await execa("pnpm", ["install", "--lockfile-only"], { | |
| cwd: projectDir, | |
| }); | |
| } else if (packageManager === "npm") { | |
| await execa("npm", ["install", "--package-lock-only"], { | |
| cwd: projectDir, | |
| }); | |
| } else if (packageManager === "bun") { | |
| // Bun doesn't have --lockfile-only, so we skip for bun | |
| // or use bun install which is fast anyway | |
| await execa("bun", ["install", "--frozen-lockfile"], { | |
| cwd: projectDir, | |
| reject: false, // Don't fail if frozen-lockfile doesn't exist | |
| }); | |
| } | |
| } catch (error) { | |
| console.error("Failed to generate lockfile:"); | |
| console.error(error); | |
| expect.fail( | |
| `Failed to generate lockfile: ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| } | |
| // Determine package manager command for running turbo | |
| const command = | |
| packageManager === "npm" | |
| ? "npx" | |
| : packageManager === "bun" | |
| ? "bunx" | |
| : "pnpm"; | |
| // Test turbo prune for both server and web targets | |
| const targets = ["server", "web"]; | |
| for (const target of targets) { | |
| const args = | |
| packageManager === "pnpm" | |
| ? ["dlx", "turbo", "prune", target, "--docker"] | |
| : ["turbo", "prune", target, "--docker"]; | |
| try { | |
| await execa(command, args, { | |
| cwd: projectDir, | |
| }); | |
| } catch (error) { | |
| console.error(`turbo prune ${target} failed:`); | |
| console.error(error); | |
| expect.fail( | |
| `turbo prune ${target} --docker failed: ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| } | |
| } | |
| } | |
| export async function validateTurboPrune(result: TestResult): Promise<void> { | |
| const { expect } = await import("vitest"); | |
| const { $ } = await import("bun"); | |
| // Extract data from result | |
| const projectDir = expectSuccessWithProjectDir(result); | |
| const config = result.config; | |
| // Only run this validation if turborepo addon is enabled | |
| const hasTurborepo = | |
| config.addons && | |
| Array.isArray(config.addons) && | |
| config.addons.includes("turborepo"); | |
| if (!hasTurborepo) { | |
| return; | |
| } | |
| const packageManager = config.packageManager || "pnpm"; | |
| // Generate lockfile without installing dependencies | |
| try { | |
| if (packageManager === "pnpm") { | |
| await $`cd ${projectDir} && pnpm install --lockfile-only`; | |
| } else if (packageManager === "npm") { | |
| await $`cd ${projectDir} && npm install --package-lock-only`; | |
| } else if (packageManager === "bun") { | |
| await $`cd ${projectDir} && bun install --frozen-lockfile`.nothrow(); | |
| } | |
| } catch (error) { | |
| console.error("Failed to generate lockfile:"); | |
| console.error(error); | |
| expect.fail( | |
| `Failed to generate lockfile: ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| } | |
| // Determine package manager command for running turbo | |
| const command = | |
| packageManager === "npm" | |
| ? "npx" | |
| : packageManager === "bun" | |
| ? "bunx" | |
| : "pnpm"; | |
| // Test turbo prune for both server and web targets | |
| const targets = ["server", "web"]; | |
| for (const target of targets) { | |
| const args = | |
| packageManager === "pnpm" | |
| ? ["dlx", "turbo", "prune", target, "--docker"] | |
| : ["turbo", "prune", target, "--docker"]; | |
| try { | |
| if (packageManager === "pnpm") { | |
| await $`cd ${projectDir} && pnpm dlx turbo prune ${target} --docker`; | |
| } else { | |
| await $`cd ${projectDir} && ${command} turbo prune ${target} --docker`; | |
| } | |
| } catch (error) { | |
| console.error(`turbo prune ${target} failed:`); | |
| console.error(error); | |
| expect.fail( | |
| `turbo prune ${target} --docker failed: ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/cli/test/test-utils.ts around lines 331 to 406, the file currently
imports and calls execa at lines ~333, 354, 358, 364 and 395; replace those
usages with Bun.$ per guidelines: remove the dynamic import of execa, keep the
vitest import, and change each execa(...) call to the equivalent Bun.$ call
using the same command and args (e.g., await Bun.$('pnpm', 'install',
'--lockfile-only') or await Bun.$('bun', 'install', '--frozen-lockfile') for
bun), pass the cwd option by prefixing with { cwd: projectDir } behavior via
Bun.$ by setting process.chdir(projectDir) before the call or by invoking Bun.$
from within a function that sets cwd (choose consistent approach used elsewhere
in the repo), and for the bun case preserve non-fatal behavior by handling the
thrown error instead of relying on reject:false (catch errors and continue when
appropriate); ensure error handling/expect.fail remains the same.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| const rootPkgJson = await readJSON(join(projectDir, "package.json")); | ||
| expect(rootPkgJson.devDependencies[configPackageName(projectName)]).toBe( | ||
| "workspace:*", |
There was a problem hiding this comment.
Bug: Test Validation Blind to Package Manager
The test validation hardcodes "workspace:*" as the expected config package dependency version, but the actual implementation uses "*" for npm and "workspace:*" for other package managers. This causes test failures when packageManager is "npm". The validation should check the package manager and expect the correct version accordingly.
|
I will look at this tomorrow morning 🫡 Can you give me a brief of what you did here? What's up with the tests? |
basically exactly what is described inside #620 Tests make sure if turbo repo is enabled they confirm that npm lock file can be generated and I'm confident with this solution as turbo repo has examples with this exact solution. https://github.com/vercel/turborepo/blob/main/examples/basic/packages/typescript-config/base.json |
|
Awesome work @moreorover 👏 |
| if (Object.keys(nativeDeps).length > 0) { | ||
| await addPackageDependency({ | ||
| customDependencies: nativeDeps, | ||
| customDevDependencies: configDep, |
There was a problem hiding this comment.
Bug: Root Package Missing Dev Dependency
The root package.json doesn't get the config package added as a dev dependency. The addPackageDependency call for projectDir is missing customDevDependencies: configDep, but the test validation at line 253-255 expects @{projectName}/config to be in the root's devDependencies. All other packages correctly receive this dependency.
2 participants
closes #620
Summary by CodeRabbit
Release Notes
New Features
Chores
Note
Adds a project-scoped
@<name>/configpackage and switches generated packages to extend it, wiring it as a devDependency; includes tests and turbo-prune validation.packages/configfromtemplates/packages/config(addspackage.json,tsconfig.base.json).setupBackendFramework.tsconfig.json.hbsinpackages/api,packages/db, auth, and server templates to extend@{{projectName}}/config/tsconfig.base.json.templates/base/tsconfig.json.hbs.@<project>/configas a devDependency where present (packages/db,packages/auth,packages/api,apps/server,apps/web,apps/native).config-package.test.tsand new helpers intest-utils.tsto validate config package wiring andturbo prune --dockeroutcomes across stack variants.Written by Cursor Bugbot for commit e3c355e. This will update automatically on new commits. Configure here.