Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Nov 3, 2025

Close #82

Summary by CodeRabbit

Release Notes

  • Chores
    • Version bumped to 2.7.0
    • Vaadin framework upgraded from 14.8.11 to 14.11.13
    • Test and runtime dependencies updated: webdrivermanager, commons-demo
    • New json-migration-helper dependency integrated
    • Jetty baseline upgraded to 11.0.26
    • Internal code refactoring for improved stability and maintenance

@javier-godoy javier-godoy requested a review from paodb November 3, 2025 16:32
@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Walkthrough

This PR addresses a Vaadin 25 compatibility issue by introducing a new json-migration-helper dependency, updating project and dependency versions (2.6.2→2.7.0, Vaadin 14.8.11→14.11.13), and refactoring ChipField.java to use JSON migration wrapper methods instead of direct type conversions between elemental.json and jackson.

Changes

Cohort / File(s) Summary
Build Configuration
.gitignore
Added /src/main/frontend/generated and /src/main/frontend/index.html to ignored paths for frontend artifacts.
Project Configuration
pom.xml
Bumped project version from 2.6.2-SNAPSHOT to 2.7.0-SNAPSHOT. Updated Vaadin from 14.8.11 to 14.11.13, webdrivermanager from 4.3.1 to 6.3.2, commons-demo from 3.5.1 to 3.10.0, and Jetty from 11.0.12 to 11.0.26 (v24 profile). Added new dependency: json-migration-helper (com.flowingcode.vaadin, version 0.0.1-SNAPSHOT).
JSON Handling Refactor
src/main/java/com/flowingcode/vaadin/addons/chipfield/ChipField.java
Replaced direct e.getEventData() calls with JsonMigration.getEventData(e) in chip event handlers. Replaced direct getElement().setPropertyJson() calls with JsonMigration.setPropertyJson() wrapper. Added import for JsonMigration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that JsonMigration.getEventData() and JsonMigration.setPropertyJson() correctly abstract the elemental.json vs jackson type conversion as a solution to the issue Vaadin 25: error during build (elemental.json vs jackson) #82 compilation errors
  • Confirm the new json-migration-helper dependency (0.0.1-SNAPSHOT) is stable and appropriately versioned for this minor release bump
  • Validate that dependency version updates (webdrivermanager 4.3.1→6.3.2, commons-demo 3.5.1→3.10.0) do not introduce breaking changes

Possibly related PRs

  • ci: upgrade v24 profile #71 — Modifies overlapping pom.xml areas (v24 profile and shared dependency entries), suggesting related parallel maintenance efforts.

Suggested reviewers

  • paodb
  • mlopezFC

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat: add json-migration-helper for compatibility with Vaadin 25' accurately describes the main change in the PR. The changeset shows the addition of a new dependency (json-migration-helper) to pom.xml, Vaadin version updates, and modifications to ChipField.java to use the new json-migration-helper utilities for handling event data and property JSON operations. The title is concise, clear, and specific enough for teammates to understand the primary purpose of the changes.
Linked Issues check ✅ Passed The pull request successfully addresses the compilation failures reported in linked issue #82. The issue described type incompatibilities between elemental.json.JsonObject and tools.jackson.databind.JsonNode at specific line numbers in ChipField.java. The PR resolves these by: (1) introducing json-migration-helper dependency to handle type conversions, (2) refactoring lines 154, 181, and 200 in ChipField.java to use JsonMigration.getEventData() and JsonMigration.setPropertyJson() instead of direct type assignments, and (3) updating the Vaadin baseline to 14.11.13 to support Vaadin 25 compatibility.
Out of Scope Changes check ✅ Passed The pull request contains several dependency updates beyond the core json-migration-helper addition. While these updates (webdrivermanager from 4.3.1 to 6.3.2, commons-demo from 3.5.1 to 3.10.0, Jetty from 11.0.12 to 11.0.26, and project version bump to 2.7.0-SNAPSHOT) are not explicitly mentioned in issue #82, they are reasonable supporting changes for establishing Vaadin 25 compatibility. The main objective of resolving the compilation errors is achieved through the json-migration-helper integration, and these dependency updates appear to be standard maintenance accompanying a new major version alignment.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vaadin25

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79e7ecc and 9e5068d.

📒 Files selected for processing (3)
  • .gitignore (1 hunks)
  • pom.xml (5 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chipfield/ChipField.java (4 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). (1)
  • GitHub Check: build-vaadin24
🔇 Additional comments (11)
.gitignore (1)

20-22: LGTM!

The added ignore patterns are standard for Vaadin projects and correctly exclude generated frontend assets and bundles from version control.

src/main/java/com/flowingcode/vaadin/addons/chipfield/ChipField.java (4)

30-30: LGTM!

The import is correctly added to support the JsonMigration wrapper methods used below.


154-156: Correctly addresses the Vaadin 25 type incompatibility.

The wrapper method JsonMigration.getEventData(e) resolves the compilation error where JsonNode cannot be converted to JsonObject.


181-183: Correctly addresses the Vaadin 25 type incompatibility.

Consistent with the fix at line 155, this wrapper resolves the JsonNode to JsonObject conversion error.


201-201: Correctly addresses the Vaadin 25 type incompatibility.

The wrapper method handles the JsonArray to BaseJsonNode conversion, resolving the third compilation error reported in issue #82.

pom.xml (6)

7-7: LGTM!

The minor version bump from 2.6.2 to 2.7.0 is appropriate for adding the new json-migration-helper dependency and Vaadin 25 compatibility.


12-12: LGTM!

Updating the baseline Vaadin 14 version from 14.8.11 to 14.11.13 is a reasonable maintenance update alongside the Vaadin 25 compatibility work.


158-158: LGTM!

The webdrivermanager test dependency update to version 6.3.2 is reasonable and test-scoped.


171-171: LGTM!

The commons-demo test dependency update to version 3.10.0 is appropriate for test scope.


480-480: LGTM!

The Jetty version update to 11.0.26 in the v24 profile is appropriate for Vaadin 24 compatibility.


128-132: Confirm release timeline for json-migration-helper with the FlowingCode team.

The SNAPSHOT artifact is accessible (HTTP 200 confirmed at the FlowingCode Snapshots repository), but using version 0.0.1-SNAPSHOT in production code still poses stability risks:

  • SNAPSHOT versions can change without notice, impacting build reproducibility
  • Version 0.0.1 indicates early/unstable code

Before merging to master, confirm with the FlowingCode team:

  1. When a stable release version (1.0.0 or higher) will be available
  2. The timeline for stabilizing this dependency

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@paodb paodb merged commit 5cbfba8 into master Nov 3, 2025
7 of 11 checks passed
@paodb paodb deleted the vaadin25 branch November 3, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vaadin 25: error during build (elemental.json vs jackson)

3 participants