-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: More lodash deps cleanup #246
Conversation
WalkthroughThe pull request introduces updates to several Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (2)packages/auth/package.json (2)
The addition of
Ensure that the catalog version of ✅ Verification successfulBased on the shell script results, I can generate the final response as we have sufficient information about the versions. The output shows that across the repository, the catalog versions resolve to:
These versions are compatible as TypeScript type definitions for lodash 4.17.x are covered by @types/lodash 4.17.x. The minor version difference (21 vs 13) is not a concern as type definitions often lag behind the main package, and the major.minor versions match (4.17). Catalog versions of lodash and @types/lodash are compatible 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check the versions of lodash and @types/lodash in the catalog
# and verify their compatibility
# Look for catalog version definitions
rg -A 1 "lodash.*catalog"
# Check package-lock.json or yarn.lock for resolved versions
fd -g '*lock*' -x cat {} \; | rg -A 5 "lodash"
Length of output: 11032 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
Documentation and Community
|
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: 1
🧹 Outside diff range and nitpick comments (1)
packages/api/package.json (1)
Inconsistent lodash dependency version found in packages/job-dispatch
Most packages use
catalog:
for bothlodash
and@types/lodash
, butpackages/job-dispatch
uses^4.17.21
forlodash
. Additionally,packages/auth
is missing@types/lodash
, andapps/pty-proxy
is missing the runtimelodash
dependency.
packages/job-dispatch/package.json
: Update"lodash": "^4.17.21"
to"lodash": "catalog:"
packages/auth/package.json
: Add"@types/lodash": "catalog:"
to devDependenciesapps/pty-proxy/package.json
: Add"lodash": "catalog:"
to dependencies if the package uses lodash runtime functions🔗 Analysis chain
Line range hint
29-29
: Verify lodash dependency consistency across all packagesLet's ensure all packages follow the same pattern for lodash dependencies.
Also applies to: 39-39, 48-48, 54-54
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check consistency of lodash dependencies across all package.json files echo "Checking lodash dependencies in all package.json files..." find . -name "package.json" -type f -exec sh -c ' echo "\nAnalyzing: {}" jq -r ". | select(.dependencies.lodash != null or .devDependencies[\"@types/lodash\"] != null) | { path: \"$(dirname {})\", lodash: .dependencies.lodash, types_lodash: .devDependencies[\"@types/lodash\"] }" {} \; ' \;Length of output: 4134
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/event-worker/package.json
(2 hunks)apps/jobs/package.json
(1 hunks)apps/pty-proxy/package.json
(1 hunks)integrations/google-compute-scanner/package.json
(1 hunks)integrations/kubernetes-job-agent/package.json
(1 hunks)integrations/terraform-cloud-scanner/package.json
(1 hunks)packages/api/package.json
(1 hunks)packages/job-dispatch/package.json
(1 hunks)pnpm-workspace.yaml
(1 hunks)
🔇 Additional comments (8)
pnpm-workspace.yaml (1)
21-21
: Consider aligning @types/lodash version with lodash
The catalog defines lodash
as ^4.17.21
but @types/lodash
is set to ^4.17.5
. To ensure type definition compatibility, consider updating @types/lodash
to match the major.minor version of lodash
.
apps/jobs/package.json (1)
Line range hint 29-33
: LGTM! Dependencies properly aligned with workspace catalog
The migration of dependencies to use catalog references is consistent with the workspace configuration and helps centralize version management.
integrations/kubernetes-job-agent/package.json (1)
40-43
: LGTM! Consistent dependency management approach
The migration to catalog references maintains consistency with the workspace configuration and other packages in the monorepo.
integrations/google-compute-scanner/package.json (1)
Line range hint 41-45
: LGTM! Consistent dependency management approach.
The changes align well with the lodash cleanup initiative. The transition to catalog references for both lodash
and @types/lodash
maintains proper type safety while centralizing version management.
Let's verify the consistency of lodash-related dependencies across the workspace:
apps/pty-proxy/package.json (1)
38-38
: Verify the necessity of @types/lodash dependency.
I notice that @types/lodash
is added as a dev dependency, but there's no direct lodash
dependency. Let's verify if this type definition is actually needed.
integrations/terraform-cloud-scanner/package.json (1)
41-41
: LGTM! Proper dependency structure maintained.
The update to use catalog reference for @types/lodash
is consistent with the runtime lodash
dependency, maintaining proper typing support while centralizing version management.
apps/event-worker/package.json (1)
29-29
: LGTM: Consistent dependency management approach
The changes to both lodash
and @types/lodash
dependencies align with the catalog-based dependency management strategy. This is consistent with other dependencies in the file and helps centralize version control.
Also applies to: 39-39
packages/api/package.json (1)
54-54
: LGTM: Consistent catalog reference usage
The update to @types/lodash
maintains consistency with the main lodash
dependency, both using catalog references. This alignment ensures proper type safety and version management.
@@ -45,7 +45,7 @@ | |||
"@ctrlplane/eslint-config": "workspace:*", | |||
"@ctrlplane/prettier-config": "workspace:*", | |||
"@ctrlplane/tsconfig": "workspace:*", | |||
"@types/lodash": "^4.17.5", | |||
"@types/lodash": "catalog:", |
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.
Align lodash runtime dependency with its type definitions
While @types/lodash
has been updated to use a catalog reference, the main lodash
dependency (line 34) still uses a fixed version ^4.17.21
. Consider updating the runtime dependency to also use a catalog reference for consistency.
Apply this change to align with the dependency cleanup initiative:
- "lodash": "^4.17.21",
+ "lodash": "catalog:",
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
@types/lodash
to the catalog inpnpm-workspace.yaml
, expanding type definitions for the lodash library.@types/lodash
and other dependencies to use catalog references in multiple package configurations.