feat: add polymer dependency for Vaadin 25 support#20
Conversation
WalkthroughThis PR updates the Vaadin framework dependency in profile v25 from version 25.0.0-beta5 to 25.0.3, adds an optional vaadin-dev dependency, declares a Polymer package dependency in GraniteAlert, excludes dynamic-theme.properties from the demo JAR build, and adds a new dynamic theme test resource configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/flowingcode/vaadin/addons/granitealert/GraniteAlert.java (1)
38-39: Consider removing or documenting the commented-out annotations.These commented-out
@NpmPackageand@JsModuleannotations for@granite-elements/granite-alertcould be confusing for future maintainers. Consider either removing them or adding a brief comment explaining why the local JS modules are used instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/granitealert/GraniteAlert.java` around lines 38 - 39, The commented-out annotations //@NpmPackage and //@JsModule for `@granite-elements/granite-alert` in the GraniteAlert class are ambiguous; either delete these two lines or replace them with a short explanatory comment above them indicating why the project uses local JS modules instead of the annotated NPM/JS module (e.g., local build, compatibility, or version reasons). Locate the commented lines referencing `@NpmPackage`(value = "@granite-elements/granite-alert", version = "^2.1.2") and `@JsModule`("@granite-elements/granite-alert/granite-alert.js") in GraniteAlert.java and apply the chosen change.pom.xml (1)
395-401: Consider adding<scope>test</scope>for thevaadin-devdependency.The
vaadin-devartifact provides development-mode tooling (hot reload, debug features). Since this is an add-on library, consumers shouldn't need this dependency transitively. While<optional>true</optional>prevents transitive inclusion, using<scope>test</scope>would more accurately reflect that this is only needed for local development/testing of the add-on itself.Suggested change
<dependencies> <dependency> <groupId>com.vaadin</groupId> <artifactId>vaadin-dev</artifactId> - <optional>true</optional> + <scope>test</scope> </dependency> </dependencies>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 395 - 401, The vaadin-dev dependency (groupId=com.vaadin, artifactId=vaadin-dev) is development-only and should be non-transitive: add a <scope>test</scope> element to the dependency declaration so it is only used for testing/local dev; you can also remove the existing <optional>true</optional> since test scope already prevents transitive inclusion, ensuring consumers of the add-on won't receive this artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pom.xml`:
- Around line 395-401: The vaadin-dev dependency (groupId=com.vaadin,
artifactId=vaadin-dev) is development-only and should be non-transitive: add a
<scope>test</scope> element to the dependency declaration so it is only used for
testing/local dev; you can also remove the existing <optional>true</optional>
since test scope already prevents transitive inclusion, ensuring consumers of
the add-on won't receive this artifact.
In `@src/main/java/com/flowingcode/vaadin/addons/granitealert/GraniteAlert.java`:
- Around line 38-39: The commented-out annotations //@NpmPackage and //@JsModule
for `@granite-elements/granite-alert` in the GraniteAlert class are ambiguous;
either delete these two lines or replace them with a short explanatory comment
above them indicating why the project uses local JS modules instead of the
annotated NPM/JS module (e.g., local build, compatibility, or version reasons).
Locate the commented lines referencing `@NpmPackage`(value =
"@granite-elements/granite-alert", version = "^2.1.2") and
`@JsModule`("@granite-elements/granite-alert/granite-alert.js") in
GraniteAlert.java and apply the chosen change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/granitealert/GraniteAlert.javasrc/test/resources/META-INF/dynamic-theme.properties
github-project-automation
bot
moved this from To Do
to Pending release
in Flowing Code Addons
2 participants
Close #18
Tested for Vaadin 14, 24 & 25.
Summary by CodeRabbit