-
-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: exclude IDE packages from regular build #1911
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifying the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
script/test-scaffold.ts (1)
22-22
: LGTM! Version constraint for zod is appropriate.The addition of
^3.22.4
for zod ensures consistent behavior across different environments while still allowing minor updates and patches.Consider also adding version constraints for
decimal.js
and@types/node
to ensure complete dependency predictability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
.github/workflows/build-test.yml
is excluded by!**/*.yml
package.json
is excluded by!**/*.json
packages/plugins/openapi/package.json
is excluded by!**/*.json
packages/plugins/trpc/package.json
is excluded by!**/*.json
packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.json
is excluded by!**/*.json
packages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.json
is excluded by!**/*.json
packages/plugins/trpc/tests/projects/t3-trpc-v10/package.json
is excluded by!**/*.json
packages/plugins/trpc/tests/projects/t3-trpc-v11/package.json
is excluded by!**/*.json
📒 Files selected for processing (1)
script/test-scaffold.ts
(1 hunks)
🔇 Additional comments (1)
script/test-scaffold.ts (1)
22-22
: Verify zod version compatibility and security.
Let's verify if this version of zod is appropriate and secure.
✅ Verification successful
Let me analyze the usage of zod in the codebase to ensure version compatibility. I'll check the specific zod features being used.
Version ^3.22.4 is appropriate and secure
The specified zod version (^3.22.4) is suitable because:
- It's close to the latest version (3.24.1) and the caret range allows safe updates
- The codebase consistently uses this version across multiple packages
- The usage patterns show standard zod features (z.union, z.object, z.number, etc.) that are stable across 3.x versions
- No breaking changes or security issues are reported between 3.22.4 and 3.24.1
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check zod version compatibility and security
# Check latest version from npm
echo "Latest zod version:"
npm view zod version
# Check for any security advisories
echo -e "\nSecurity advisories for zod:"
npm audit zod@^3.22.4 || true
# Check if any other files in the project use zod to verify compatibility
echo -e "\nFiles using zod:"
rg -l "from ['|\"]zod['|\"]" || true
Length of output: 1488
Script:
#!/bin/bash
# Check how zod is being used in the codebase to verify compatibility
echo "Checking zod usage patterns:"
rg "from ['|\"]zod['|\"]" -B 1 -A 3
# Check package.json files for any zod version constraints
echo -e "\nChecking package.json files for zod dependencies:"
fd package.json --exec cat {} | grep -A 1 -B 1 "zod"
Length of output: 10675
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/runtime/src/zod-utils.ts (2)
12-13
: Documentation could be more specific about version compatibility.While the added documentation explains the use of
any
, it would be helpful to specify which Zod versions this change aims to support.Consider expanding the comment to include:
- * The function uses `any` for parameter and return type to be compatible with various zod versions. + * The function uses `any` for parameter and return type to be compatible with Zod versions ^3.22.4 and above, + * trading type safety for broader version compatibility.
31-33
: Consider a more consistent type safety approach.There's an inconsistency between using
any
in the function signature but maintaining specific Zod type assertions internally. Consider one of these approaches:
- Use type parameters to maintain type safety while ensuring version compatibility
- Document why specific type assertions are needed despite using
any
typesHere's a potential type-safe alternative using generics:
export function smartUnion<T extends Z.ZodTypeAny>( z: typeof Z, candidates: T[] ): Z.ZodType<T['_output']> { // ... rest of the implementation }Also applies to: 43-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime/src/zod-utils.ts
(3 hunks)
🔇 Additional comments (1)
packages/runtime/src/zod-utils.ts (1)
15-15
: Verify if type relaxation is necessary for all supported Zod versions.
While using any
types provides maximum compatibility, it significantly reduces type safety. Let's verify if this change is necessary for all supported Zod versions.
No description provided.