Conversation
- Introduced a public type helper for TypeScript projects to provide typesafe, autocompleted access to VITE_* environment variables. - Updated Vite plugin to expose schema-aware typings, allowing client environment types to be derived from the Env schema. - Added documentation on typing import.meta.env with ArkEnv schemas and improved clarity in Vite plugin documentation.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 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. WalkthroughThis PR establishes a new internal types package ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
yamcodes
changed the title
- Introduced a new internal types package located at `packages/internal/types/` to export common TypeScript types. - Updated references to the new package name `@repo/types` in relevant files and documentation. - Simplified package structure by removing unnecessary build steps, as it is a types-only package. - Ensured that the package is not published to npm and is intended for internal use only within the monorepo.
- Added `@repo/types` dependency to `packages/arkenv` and `packages/vite-plugin` to utilize shared TypeScript types from the new internal types package. - Updated relevant files to import `InferType` from `@repo/types`, removing duplicate type definitions in both packages. - Modified `pnpm-workspace.yaml` to include the new internal types package and updated `.changeset/config.json` to ignore it during versioning.
github-actions
bot
added
arkenv
📦 Bundle Size Report
✅ All size limits passed! |
commit: |
- Removed `@repo/types` from the ignore list in `.changeset/config.json` to ensure it is included in versioning updates.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/arkenv/package.json
Outdated
| "dependencies": { | ||
| "@repo/types": "workspace:*" | ||
| }, |
There was a problem hiding this comment.
The arkenv package now declares @repo/types as a dependency. That package is marked private: true in packages/internal/types/package.json and is ignored from releases, so it will not be published to npm. Publishing arkenv with this dependency means any install outside the monorepo will attempt to resolve @repo/types from the registry and fail, breaking consumers’ installs and type-checking. Consider inlining the type or publishing a public types package instead.
Useful? React with 👍 / 👎.
| "dependencies": { | ||
| "@repo/types": "workspace:*", | ||
| "arkenv": "workspace:*" |
There was a problem hiding this comment.
The Vite plugin adds a dependency on @repo/types, but that package is private and not published. When @arkenv/vite-plugin is installed from npm, npm will try to fetch @repo/types and fail because it only exists in the workspace, breaking plugin installs for users. The types should be bundled or exposed through a published package rather than depending on an unpublished workspace module.
Useful? React with 👍 / 👎.
- Changed `arkenv` dependency in `apps/playgrounds/node/package.json` to use a specific tarball file instead of workspace reference. - Added `@repo/types` as a dependency in `packages/arkenv` and `packages/vite-plugin` to ensure consistent type usage across packages. - Updated `tsdown.config.ts` files in both `packages/arkenv` and `packages/vite-plugin` to resolve types from `@repo/types`.
- Updated the `InferType` definition in `packages/internal/types/index.ts` to enhance type inference from ArkType definitions. - Changed the output path for type definitions in `packages/internal/types/package.json` to `./dist/index.d.ts`. - Modified `tsconfig.json` in `packages/internal/types` to enable declaration file generation and set the output directory. - Removed unused `dts` configuration from `packages/vite-plugin/tsdown.config.ts`. - Adjusted import statements in `packages/vite-plugin/src/types.ts` for consistency.
![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 (2)
packages/vite-plugin/package.json (1)
15-15: Critical: Unpublished dependency will break external installs.The vite-plugin package depends on
@repo/types, which is marked as private and will not be published to npm. When users install@arkenv/vite-pluginfrom npm, the installation will fail because npm cannot resolve@repo/types.Even as a devDependency, if the build process doesn't properly bundle or inline the types from
@repo/types, the published package will have broken type references. Verify that the build tool (tsdown) is configured to bundle all type dependencies into the output.Run the following script to verify tsdown configuration:
#!/bin/bash # Check tsdown config for type bundling echo "=== Vite Plugin tsdown.config.ts ===" cat packages/vite-plugin/tsdown.config.ts echo -e "\n=== Check if @repo/types is in dts.resolve ===" rg -A 5 'dts.*resolve' packages/vite-plugin/tsdown.config.tspackages/arkenv/package.json (1)
44-44: Critical: Unpublished dependency will break external installs.The arkenv package depends on
@repo/types, which is marked as private and will not be published to npm. When users installarkenvfrom npm, the installation will fail because npm cannot resolve@repo/types.As a devDependency, if the build process doesn't properly bundle or inline the types from
@repo/typesinto the published output, the package will have broken type references. Verify that tsdown is configured to bundle all type dependencies.Run the following script to verify tsdown configuration:
#!/bin/bash # Check tsdown config for type bundling echo "=== ArkEnv tsdown.config.ts ===" cat packages/arkenv/tsdown.config.ts echo -e "\n=== Check if @repo/types is in dts.resolve ===" rg -A 5 'dts.*resolve' packages/arkenv/tsdown.config.ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/playgrounds/node/package.json(1 hunks)openspec/changes/add-internal-types-package/design.md(1 hunks)openspec/changes/add-internal-types-package/proposal.md(1 hunks)openspec/changes/add-internal-types-package/specs/internal-types/spec.md(1 hunks)openspec/changes/add-internal-types-package/tasks.md(1 hunks)packages/arkenv/package.json(1 hunks)packages/arkenv/src/create-env.ts(1 hunks)packages/arkenv/tsdown.config.ts(1 hunks)packages/internal/types/README.md(1 hunks)packages/internal/types/index.ts(1 hunks)packages/internal/types/infer-type.ts(1 hunks)packages/internal/types/package.json(1 hunks)packages/internal/types/tsconfig.json(1 hunks)packages/vite-plugin/package.json(1 hunks)packages/vite-plugin/src/types.ts(1 hunks)packages/vite-plugin/tsdown.config.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-10T19:35:18.179Z
Learnt from: yamcodes
Repo: yamcodes/arkenv PR: 136
File: .changeset/vast-bananas-win.md:2-3
Timestamp: 2025-09-10T19:35:18.179Z
Learning: The arkenv package is currently in v0.x.x (pre-1.0) development phase, where breaking changes are acceptable in minor version bumps according to semantic versioning conventions.
Applied to files:
packages/arkenv/package.jsonapps/playgrounds/node/package.json
🧬 Code graph analysis (1)
apps/playgrounds/node/package.json (1)
packages/vite-plugin/src/index.ts (1)
arkenv(50-86)
🪛 LanguageTool
openspec/changes/add-internal-types-package/tasks.md
[uncategorized] ~20-~20: The official name of this software platform is spelled with a capital “H”.
Context: ...## 4. Update Workflows - [x] 4.1 Review .github/workflows/size-limit.yml to ensure `pa...
(GITHUB)
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...ered by packages/**) - [x] 4.2 Review .github/workflows/pkg-pr-new.yml to ensure int...
(GITHUB)
[uncategorized] ~22-~22: The official name of this software platform is spelled with a capital “H”.
Context: ...ould not be published) - [x] 4.3 Review .github/workflows/test.yml to ensure internal ...
(GITHUB)
openspec/changes/add-internal-types-package/design.md
[style] ~82-~82: ‘for the benefit’ might be wordy. Consider a shorter alternative.
Context: ...organization, but it's a small overhead for the benefit of eliminating duplication - **Internal...
(EN_WORDINESS_PREMIUM_FOR_THE_BENEFIT)
⏰ 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). (3)
- GitHub Check: test-e2e (e2e)
- GitHub Check: test-e2e (a11y)
- GitHub Check: test-build (latest)
🔇 Additional comments (15)
packages/internal/types/README.md (1)
1-28: LGTM! Clear and comprehensive documentation.The README effectively documents the internal types package's purpose, usage patterns, and important constraints. The emphasis on internal-only usage and workspace protocol dependencies is appropriate.
packages/internal/types/tsconfig.json (1)
1-18: LGTM! Appropriate TypeScript configuration for a types-only package.The configuration correctly uses
emitDeclarationOnly: truefor a types-only package, and the strict mode settings align with best practices.openspec/changes/add-internal-types-package/specs/internal-types/spec.md (1)
1-32: LGTM! Well-structured specification.The specification clearly defines requirements and scenarios for the internal types package, including proper handling in CI/CD workflows and workspace-only usage.
pnpm-workspace.yaml (1)
3-3: LGTM! Workspace configuration updated correctly.Adding
packages/internal/*to the workspace appropriately includes the new internal types package in the monorepo's workspace configuration.packages/internal/types/index.ts (1)
1-8: LGTM! Clean entry point for the internal types package.The simple re-export pattern is appropriate for a types-only package, and the header comment clearly documents the internal-only nature of the package.
packages/arkenv/tsdown.config.ts (1)
7-9: LGTM!The
dts.resolveconfiguration correctly enables tsdown to resolve type declarations from the internal@repo/typespackage when generating declaration files.openspec/changes/add-internal-types-package/design.md (1)
1-97: LGTM!The design document provides clear rationale for introducing the internal types package, covers alternatives considered, and outlines the migration plan. The decision to use workspace protocol and keep the package unpublished is appropriate for an internal utility.
packages/vite-plugin/src/types.ts (1)
1-1: LGTM!Successfully migrated from local
InferTypedefinition to importing from the centralized@repo/typespackage.packages/vite-plugin/tsdown.config.ts (1)
7-9: LGTM!The
dts.resolveconfiguration is correctly set up to resolve the internal@repo/typespackage during declaration file generation.packages/internal/types/infer-type.ts (1)
1-17: LGTM!The
InferTypeutility correctly extracts the inferred type from ArkType definitions by handling both function call signatures (returning validated values) and direct type definitions (viatype.Any).packages/arkenv/src/create-env.ts (1)
1-1: LGTM!Successfully migrated from local
InferTypedefinition to importing from the centralized@repo/typespackage. The type is used consistently across the function overloads.openspec/changes/add-internal-types-package/proposal.md (1)
1-32: LGTM!The proposal clearly articulates the rationale for introducing the internal types package and correctly identifies it as an internal refactoring with no breaking changes.
packages/internal/types/package.json (2)
18-24: arktype versions are consistent and compatible across the workspace.All three packages (arkenv, internal/types, vite-plugin) specify the same devDependency version
2.1.27and peerDependency range^2.1.22. Version 2.1.27 satisfies the peerDependency constraint, ensuring no type mismatches or compatibility issues.
9-13: Exports configuration is correct as-is.The current configuration using only the
"types"condition is the appropriate pattern for a types-only package. The tsconfig.json'semitDeclarationOnly: truesetting ensures only .d.ts files are generated with no JavaScript output, making additional"import"or"default"conditions unnecessary and misleading. The monorepo workspace configuration properly resolves@repo/typesin consuming packages (arkenv and vite-plugin), confirming the setup works correctly.openspec/changes/add-internal-types-package/tasks.md (1)
1-36: Well-structured task checklist; verify actual implementation completion.The task list comprehensively documents the internal types package initiative with clear sections covering structure, type extraction, consumer updates, CI/CD workflows, testing, and documentation. The logical ordering and granular breakdown make it easy to track the implementation scope.
However, this is a checklist of completed tasks rather than implementation artifacts. All items are marked
[x]complete, but the actual implementation files (new internal package structure, updated imports, modified configurations, etc.) need verification to confirm alignment with the documented tasks.To verify the tasks were completed as documented, please confirm:
- The internal package structure exists at
packages/internal/types/withindex.ts,infer-type.ts, and supporting config files- Consumer packages (
arkenvandvite-plugin) importInferTypefrom@repo/typesinstead of local definitions- Workflow files properly handle the internal package (type-check only, excluded from publishing)
pnpm-workspace.yamlincludespackages/internal/*and changesets config excludes@repo/types- Documentation in the internal package README clearly indicates non-published status and usage guidelines
- Changed the `arkenv` dependency in both `apps/playgrounds/node/package.json` and `pnpm-lock.yaml` to use a workspace reference instead of a specific version, facilitating local development and consistency across packages.
1 participant
Closes #416
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.