fix: ensure latest compression-webpack-plugin is installed#1035
fix: ensure latest compression-webpack-plugin is installed#1035justin808 merged 3 commits intoshakacode:mainfrom
compression-webpack-plugin is installed#1035Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR aligns Key points:
Confidence Score: 5/5Safe to merge — tiny, correct one-line fix with no logic changes. The change is a straightforward synchronization of a version range string that was already declared in package.json peerDependencies. No logic is altered. The only open checklist item (CHANGELOG) is a documentation housekeeping task flagged by the author themselves and does not affect correctness. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[rails shakapacker:install] --> B[template.rb reads lib/install/package.json]
B --> C{SHAKAPACKER_ASSETS_BUNDLER}
C -->|webpack| D[Fetch 'webpack' group]
C -->|rspack| E[Fetch 'rspack' group]
D --> F[Fetch 'common' group]
E --> F
F --> G["compression-webpack-plugin:\n^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0"]
G --> H{version starts with ^ or ~?}
H -->|yes| I["Use version string as-is"]
H -->|no| J["Extract last major version number"]
I --> K[package_manager.add! production deps]
J --> K
K --> L[npm/yarn installs latest matching version → v12.x.x]
Reviews (1): Last reviewed commit: "docs: add changelog entry" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/install/package.json`:
- Line 18: The docs still list compression-webpack-plugin support only up to
^11.0.0 while package.json now allows ^12.0.0; update docs/peer-dependencies.md
to include ^12.0.0 for compression-webpack-plugin (or change the wording to
reflect support for ^9–^12) so the documented peer-dependency range matches the
"compression-webpack-plugin" entry in package.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 559505bd-6420-476a-92a2-d2444698734f
📒 Files selected for processing (1)
lib/install/package.json
|
@G-Rath thank you! |
### Summary <!-- Describe the code changes in your pull request here - were there any bugs you had fixed, features you added, tradeoffs you made? If so, mention them. If these changes have open GitHub issues, tag them here as well to keep the conversation linked. --> The current logic ends up passing the full constraint range which TIL is actually valid, but still confusing and ultimately this can be simplified. ### Pull Request checklist <!-- If any of the items on this checklist do not apply to the PR, both check it out and wrap it by `~`. --> - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] ~Update CHANGELOG file~ ### Other Information <!-- Mention any other important information that might relate to this PR but that don't belong in the summary, like detailed benchmarks or possible follow-up changes. --> I came across this while checking out #1035 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved how dependency version constraints are processed during package installation, ensuring more consistent handling of complex version specifications. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
While v12 is allowed as a peer dependency, the install script currently does not use the latest version.
Pull Request checklist
Add/update test to cover these changesUpdate documentationOther Information
fwiw I think there's a few ways this could be improved, e.g.:
node_modules/shakapacker/package.jsondirectly to get the version ranges (so this file would just hold arrays for the groupings)Summary by CodeRabbit