Repository navigation
refactor: declare the npm packages only in GoogleMap - #188
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Remove component-level declarations src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapAdvancedMarker.java, src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapMarker.java, src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoint.java, src/main/java/com/flowingcode/vaadin/addons/googlemaps/GoogleMapPoly.java |
The four component classes no longer declare the two npm package versions. Their JavaScript module declarations remain. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Possibly related PRs
- FlowingCode/GoogleMapsAddon#186: Adds the same package annotations to
GoogleMapAdvancedMarkerand updates them in existing marker classes. - FlowingCode/GoogleMapsAddon#138: Updates the
@flowingcode/google-mapdependency annotation in several of the affected component classes. - FlowingCode/GoogleMapsAddon#147: Updates the same dependency annotation in the affected component classes.
Suggested reviewers: javier-godoy
Merge Risk: 
ab2bd
-
The package declarations remain discoverable after centralization. No actionable merge risk was identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: moving the npm package declarations so they exist only in GoogleMap. Linked Issues check Issue #187asks that each npm package version be declared in one place. The diff from merge base to head removes the duplicate@NpmPackagedeclarations for@flowingcode/google-map(3.10.1) and@google…Out of Scope Changes check The whole-PR diff touches four files, all in src/main/java/com/flowingcode/vaadin/addons/googlemaps. Each change removes only @NpmPackageannotations, and the matching NpmPackage import where it becam…Full details: Linked Issues check
-
Explanation
Issue
#187asks that each npm package version be declared in one place. The diff from merge base to head removes the duplicate@NpmPackagedeclarations for@flowingcode/google-map(3.10.1) and@googlemaps/markerclusterer(2.6.2) from GoogleMapMarker, GoogleMapAdvancedMarker, GoogleMapPoint, and GoogleMapPoly. At head, GoogleMap.java keeps both declarations with the same versions (@NpmPackage(value = "@flowingcode/google-map", version = "3.10.1") and@NpmPackage(value = "@googlemaps/markerclusterer", version = "2.6.2")). No version is changed, and the remaining@JsModuledeclarations stay in place. This meets the single-location objective that the issue names as its first option. The issue also asks for a check of dependency resolution in development and production mode. That check is a manual verification step and is not a coding requirement, so it is not assessed here. The PR does not add automated tests for it. The claim that Vaadin collects@NpmPackagefrom all annotated classes on the classpath is the author's statement; I did not verify it against Vaadin sources, but the retained annotations on GoogleMap do not depend on that claim.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff touches four files, all in src/main/java/com/flowingcode/vaadin/addons/googlemaps. Each change removes only
@NpmPackageannotations, and the matching NpmPackage import where it became unused. The removed declarations duplicate those kept in GoogleMap, which is the change#187requests. No functional code, public API, or unrelated file changes appear in the diff. No test or documentation changes are present, and none are required for this scope.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
github-project-automation
Bot
moved this from To Do
to Pending release
in Flowing Code Addons
The
@NpmPackageannotations for@flowingcode/google-mapand@googlemaps/markerclustererwere repeated inGoogleMapMarker,GoogleMapAdvancedMarker,GoogleMapPointandGoogleMapPoly. They are now declared only inGoogleMap, so a version update changes a single class.Vaadin collects
@NpmPackagefrom every annotated class in the classpath (FrontendDependencies.computePackages, in Vaadin 14 and 25), not only from the classes reachable from routes, so the packages are still installed.Close #187
Summary by CodeRabbit