-
-
Notifications
You must be signed in to change notification settings - Fork 912
fix(packages): remove effect from optional peer dependencies #2527
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
Conversation
Fixes ERROR: Could not resolve "effect"
🦋 Changeset detectedLatest commit: d0e14a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Walkthrough
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
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
🧹 Nitpick comments (2)
.changeset/brave-roses-cheat.md (1)
2-5: Expand the changeset to reflect all peerDependency removals.The package.json also removes zod and yup from peerDependencies. Reflect that here to avoid confusing consumers reading release notes.
Apply:
---- -"@trigger.dev/schema-to-json": patch ---- - -remove effect from optional peer dependencies since it's now a production dependency +--- +"@trigger.dev/schema-to-json": patch +--- + +Remove effect, zod, and yup from optional peerDependencies. effect is now a production dependency; zod is a direct dependency (for zod-to-json-schema); yup is dev-only (tests).packages/schema-to-json/package.json (1)
43-49: Confirm semver strategy for production deps (effect).effect is now a prod dependency with range ^3. Ensure this aligns with repo policy (others like zod are pinned). If you want reproducibility, consider pinning or using an internal resolution strategy.
Would you like me to check repository-wide conventions and open a follow-up PR to normalize dependency ranges?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/brave-roses-cheat.md(1 hunks)packages/schema-to-json/package.json(1 hunks)
⏰ 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). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/schema-to-json/package.json (3)
68-84: peerDependenciesMeta correctly mirrors remaining peers.Meta entries align with current peerDependencies after removals. LGTM.
46-49: No peerDependency on "yup" in @sodaru/yup-to-json-schema — no action required.
Upstream package.json lists "yup" only in devDependencies (^1.1.1), not as a peerDependency.
61-67: OK — no runtime 'yup' imports found in packages/schema-to-json
Searched packages/schema-to-json/src for static (value) imports, require(...), and dynamic import(...): no value imports of "yup" found; no "zod" imports detected.
Fixes ERROR: Could not resolve "effect"