Merged
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds TypeScript codegen plugins to buf.gen.yaml, introduces an internal ESM package @unkey/proto with tsconfig and re-exports of generated proto artifacts, and updates the dashboard app package.json to depend on Connect libraries and the workspace @unkey/proto package. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Developer
participant Buf as buf (codegen)
participant Gen as Generated (../internal/proto/generated)
participant ProtoPkg as @unkey/proto
participant Dashboard as apps/dashboard
Developer->>Buf: run buf generate (go + TS plugins)
Buf-->>Gen: write TS artifacts (target=ts)
Gen->>ProtoPkg: included by package (src/index.ts re-exports)
Dashboard->>ProtoPkg: import @unkey/proto (workspace)
Dashboard->>Dashboard: use Connect clients/types
note right of Buf#cccccc: New TS plugins added\n(bufbuild/es, connectrpc/es)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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 |
chronark
requested review from
Flo4604,
MichaelUnkey,
imeyer,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
Contributor
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![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 (1)
apps/dashboard/package.json (1)
100-101: Duplicate TypeScript versions in deps vs devDeps.You have typescript in dependencies (^5.7.3) and devDependencies (^5.1.3). Keep a single devDependency and align versions.
"dependencies": { - "typescript": "^5.7.3", "usehooks-ts": "^3.1.0", "vaul": "^0.9.0", "zod": "^3.23.5" }, "devDependencies": { @@ - "typescript": "^5.1.3", + "typescript": "^5.7.3", "vitest": "^1.6.1" }Also applies to: 117-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (26)
internal/proto/generated/proto/ctrl/v1/acme_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/acme_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/build_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/build_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/deployment_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/deployment_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/openapi_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/openapi_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/routing_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/routing_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/service_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/ctrl/v1/service_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/assetmanagerd/v1/asset_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/assetmanagerd/v1/asset_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/billaged/v1/billing_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/billaged/v1/billing_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/builderd/v1/builder_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/deploy/builderd/v1/builder_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/metal/vmprovisioner/v1/vmprovisioner_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/metal/vmprovisioner/v1/vmprovisioner_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/metal/vmprovisioner/v1/wip_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/partition/v1/gateway_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/vault/v1/object_pb.tsis excluded by!**/generated/**internal/proto/generated/proto/vault/v1/service_connect.tsis excluded by!**/generated/**internal/proto/generated/proto/vault/v1/service_pb.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/dashboard/package.json(2 hunks)go/buf.gen.yaml(1 hunks)internal/proto/package.json(1 hunks)internal/proto/src/index.ts(1 hunks)internal/proto/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
internal/proto/src/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
internal/proto/src/index.ts
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
internal/proto/src/index.ts
🧠 Learnings (3)
📚 Learning: 2025-08-04T07:44:39.438Z
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.{ts,tsx} : Follow strict TypeScript configuration
Applied to files:
internal/proto/tsconfig.json
📚 Learning: 2025-09-01T02:33:43.749Z
Learnt from: imeyer
PR: unkeyed/unkey#3899
File: go/buf.gen.yaml:0-0
Timestamp: 2025-09-01T02:33:43.749Z
Learning: In the unkeyed/unkey repository, buf commands are executed from the `go/` directory where the `buf.yaml` file is located. This means the `out: gen` configuration in `go/buf.gen.yaml` generates files to `go/gen/` relative to the repository root, which aligns with the expected directory structure.
Applied to files:
go/buf.gen.yaml
📚 Learning: 2025-09-01T01:57:42.213Z
Learnt from: imeyer
PR: unkeyed/unkey#3899
File: go/proto/metald/v1/metald.proto:5-9
Timestamp: 2025-09-01T01:57:42.213Z
Learning: In the unkeyed/unkey repository, buf is configured to properly resolve metald proto imports like "metald/v1/vm.proto" without needing the full "go/proto/" prefix. The buf lint command `buf lint --path proto/metald` passes successfully with these relative import paths.
Applied to files:
go/buf.gen.yaml
⏰ 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). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Packages / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
🔇 Additional comments (1)
internal/proto/package.json (1)
1-22: Unify on Connect v2 and prepare @unkey/proto for v2 codegen
- Remove
@connectrpc/protoc-gen-connect-esper Connect v2 migration (plugin removed) (github.com)- Bump RPC codegen/runtime to v2:
@bufbuild/protobuf&@bufbuild/protoc-gen-es→^2.2.0(npmjs.com);@connectrpc/connect→^2.0.2(npmjs.com)- Point
"main"todist/index.js,"types"todist/index.d.ts, add"exports"&"files"for proper module resolution- Add
gen,build&cleanscripts; verifycd ../go && buf generatepath matches your project layout [verify]
Comment on lines
+1
to
+3
| // Re-export ctrl service types and services | ||
| export * from "../generated/proto/ctrl/v1/deployment_pb"; | ||
| export * from "../generated/proto/ctrl/v1/deployment_connect"; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove v1 connect-generated exports if standardizing on Connect v2.
With v2, services come from the _pb module; there is no _connect file. (github.com, connectrpc.com)
// Re-export ctrl service types and services
export * from "../generated/proto/ctrl/v1/deployment_pb";
-export * from "../generated/proto/ctrl/v1/deployment_connect";🤖 Prompt for AI Agents
In internal/proto/src/index.ts around lines 1 to 3, the file is re-exporting
both the generated protobuf types and the Connect v1 service bindings; when
standardizing on Connect v2 the _connect exports must be removed because v2
exposes services from the _pb modules. Remove the export line for
"../generated/proto/ctrl/v1/deployment_connect" (or replace any usage with the
corresponding symbols from "../generated/proto/ctrl/v1/deployment_pb") so only
the _pb module is re-exported and update any import sites to consume the
v2-style service types from the _pb exports.
Flo4604
approved these changes
Sep 1, 2025
…ore_add_proto_for_typescript
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
2 participants
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.
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit