-
Notifications
You must be signed in to change notification settings - Fork 2
fix: Use Uses instead of NpmPackage to avoid version conflicts #140
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
Updated NpmPackage annotation to ensure compatibility with the latest version.
WalkthroughReplaced an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java (1)
28-28: Remove unused import.The
NpmPackageimport is no longer used since the annotation was removed. Clean up by removing this import.Apply this diff to remove the unused import:
-import com.vaadin.flow.component.dependency.NpmPackage;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java(1 hunks)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java
Show resolved
Hide resolved
|
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 (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java (1)
39-40: Excellent approach to avoid version conflicts.Using
@Uses(Grid.class)instead of directly pinning@NpmPackage(version = "24.3.2")allows Grid to manage the@vaadin/polymer-legacy-adapterversion automatically. This ensures the dependency version stays in sync with the Grid component version, preventing conflicts as Vaadin is upgraded.Based on past review comments, Grid.class provides this dependency across multiple Vaadin versions (23.3, 23.6, 24.0, 24.9), so this approach is validated.
Optional: Consider clarifying the comment.
The comment is clear, but you could make it slightly more explicit:
-// Indirectly ensure @NpmPackage(value = "@vaadin/polymer-legacy-adapter", version = "x.x.x") +// Grid.class provides @NpmPackage for @vaadin/polymer-legacy-adapter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java(1 hunks)
🔇 Additional comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridRadioSelectionColumn.java (1)
24-24: LGTM! Import necessary for the @uses annotation.The Grid import is required for the
@Uses(Grid.class)annotation added below and is correctly placed.



Updated NpmPackage annotation to ensure compatibility with the latest version.
Summary by CodeRabbit
Bug Fixes
Performance
Chores