CAMEL-23636: add camel-diagram web component (vanilla JS, no npm)#24064
Open
ammachado wants to merge 17 commits into
Open
CAMEL-23636: add camel-diagram web component (vanilla JS, no npm)#24064ammachado wants to merge 17 commits into
ammachado wants to merge 17 commits into
Conversation
davsclaus
reviewed
Jun 17, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thank you for this well-crafted feature — the layout engine, theming, ARIA accessibility, and license handling are all nicely done.
I have one significant concern and a few observations:
Can Node.js/npm be avoided entirely?
This PR introduces frontend-maven-plugin with Node.js v22.14.0 and npm 10.9.2 as a build dependency — a first for the Camel project. Node and npm are notorious for pulling in a large transitive dependency tree with frequent CVEs, which creates ongoing maintenance burden (triaging, bumping, verifying). Even though these are build-time only, they still show up in security scans and require attention.
Could the web component be implemented as a single self-contained HTML/JS file with embedded JavaScript instead? The Lit dependency adds convenience but also pulls in npm. The component is already relatively compact (~290 lines of source JS + ~210 lines of layout logic). A vanilla Web Components implementation using HTMLElement + attachShadow + template literals would eliminate the entire Node.js/npm/esbuild/Lit toolchain and avoid the CVE treadmill. The resulting file could be committed directly as a static resource with no build step at all.
If Lit is preferred for developer ergonomics, an alternative would be to vendor a pre-built Lit bundle as a static file and import it, but the cleanest path is vanilla JS with no external dependencies.
Other observations
-Dquickly does not skip the frontend build. The PR adds a maven-test-skip profile that sets skipFrontendBuild=true, but -Dquickly will still run install-node-and-npm, npm ci, and npm run build. Since the bundle is committed to source, the frontend build is redundant on quick builds. Consider tying skipFrontendBuild to the quickly property as well.
PR description mentions THIRD-PARTY-NOTICES.txt test coverage that doesn't exist. The WebComponentBundleTest only verifies the JS bundle (existence, size, custom element registration) — there is no assertion on THIRD-PARTY-NOTICES.txt content, despite the PR body claiming it.
This review covers project rules, conventions, and code correctness visible from the diff. It does not replace specialized review tools (CodeRabbit, Sourcery) or static analyzers (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
oscerd
reviewed
Jun 17, 2026
oscerd
left a comment
oscerd
left a comment
Contributor
There was a problem hiding this comment.
This is a high-quality addition — the layout engine has real unit tests, the bundled third-party deps are properly handled (Lit BSD-3-Clause, Lucide ISC, both ASF Category-A with full texts in THIRD-PARTY-NOTICES.txt), and the component docs + upgrade-guide entry are in place.
One architectural point worth committer discussion before merge: this introduces the first frontend-maven-plugin usage in the Camel reactor. The npm-install/npm-build executions are gated only on maven.test.skip, so a plain mvn install — and notably the ASF release profile (which sets skipTests but not maven.test.skip) — will download Node v22.14.0 and npm packages from the network during the build. That raises (a) reproducible/offline source-release builds, (b) Node availability on the s390x/ppc64le build agents, and (c) CI time. Since the pre-built bundle is already committed, would it make sense to gate the frontend build behind a profile that's off for release?
Minor: the PR description says WebComponentBundleTest asserts that THIRD-PARTY-NOTICES.txt mentions Lit and Lucide, but the test only checks the JS bundle's presence/size/custom-element — either add that assertion or tweak the description.
Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
ammachado
changed the title
davsclaus
reviewed
Jun 17, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Nice work on replacing the Lit/npm toolchain with a vanilla Web Component — the result is clean and self-contained.
A few observations (all non-blocking):
-
NODE_H discrepancy (cosmetic): The JS layout engine uses
NODE_H = 36while the JavaRouteDiagramLayoutEngineusesDEFAULT_NODE_HEIGHT = 32(before 2x scale). Since these are independent renderers targeting different contexts (browser CSS pixels vs PNG), this is fine — just worth noting for future synchronization. -
.mjslicense style in root pom.xml: The PR adds<mjs>SLASHSTAR_STYLE</mjs>to the license-maven-plugin config, but no.mjsfiles are introduced. Harmless and proactively useful, but slightly out of scope. -
Security: The JS properly escapes all rendered text through
esc()before inserting intoinnerHTML(within Shadow DOM), usesAbortControllerfor fetch cancellation, and doesn't evaluate untrusted input as code. Looks clean. -
Test coverage:
RouteDiagramLayoutEngineTest(8 tests) andWebComponentBundleTest(4 tests) provide solid coverage with AssertJ. -
Documentation: Upgrade guide entry and component doc page are well-structured, with correct
xref:linking per project conventions.
Looks good overall — pending CI completion.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
…omponent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gine Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… rendering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…moke-test Dark mode media query now sets background (#0f172a) alongside text color so light text is never rendered on a transparent/white host. The smoke-test switches to an inline module import to avoid the file:// CORS restriction that blocks external <script type="module" src="..."> in Safari; also adds explicit forced light/dark sections so both themes can be verified independently of the OS color scheme. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… src/test/resources/ The relative import ../../main/resources/... goes two levels up from the HTML file's location. When Python serves from src/test/resources/ the path escapes the server root and 404s. Serving from src/ makes the path resolve correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hains In a chain [route → from → log → choice], all three were children of route in the tree, so three edges originated from the route box — the edge to log passed through from, the edge to choice passed through from and log. Added visualParentId() and lastChainId() to port RouteDiagramLayoutEngine's findLastLayoutNode() logic: for non-first children of a linear parent the edge now originates from the last node of the previous sibling's chain. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The page tests frontend source and should live next to it. From there the bundle import is a clean one-level-up path (../resources/...) and the server command is simply: cd src/main/frontend && python3 -m http.server 8080 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Wire Vitest into Maven test phase via frontend-maven-plugin (tests were previously never executed in CI) - Switch npm install to npm ci for reproducible, lockfile-exact builds - Add legalComments: 'external' to esbuild config; emit sidecar camel-route-diagram.js.LEGAL.txt with extracted Lit BSD-3-Clause notices - Add THIRD-PARTY-NOTICES.txt with full Lit BSD-3-Clause attribution text - Move .mjs -> SLASHSTAR_STYLE mapping from module pom to root pluginManagement so future frontend modules inherit it automatically - Remove redundant module-level <mapping> block from camel-diagram/pom.xml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use xref:components:others:diagram.adoc instead of the non-existent xref:components::diagram-component.adoc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JS correctness: - Fix arrowhead markers invisible in Firefox: move <defs><marker> inside each per-route <svg> so url(#id) paint-server references resolve within the same SVG element across all browsers - Add updated(changedProperties) lifecycle hook so refresh/src/filter changes after connection restart the poll timer and re-fetch; connectedCallback now calls requestUpdate() to handle DOM reconnect - Add AbortController to #doFetch() to cancel in-flight requests and prevent stale responses from overwriting fresher data - Trim src before URL construction and the empty-src guard to reject whitespace-only values - Validate that the response JSON contains a routes array; surface a clear error instead of silently rendering a blank diagram JS layout: - Fix O(n²) subtreeMaxY: assignPositions now returns the subtree bottom Y, eliminating the recursive re-traversal for each sibling in linear chains - Replace Math.max(...spread) with reduce in computeSubtreeWidth to prevent RangeError on very large child arrays - Skip nodes with no id in buildTree and assignPositions to prevent positions[undefined] key collisions Testing: - Add bundledJsContainsCustomElementRegistration() Java test to verify the bundle registers the custom element, not just that the file is non-empty - Add layout.test.js cases for: subtreeMaxY correctness (sibling placed below deepest descendant), lastChainId at branching EIP predecessor, assignPositions return value, and nodes-without-id skip behaviour Build: - Add skipFrontendBuild property (default false) with <skip> on npm-install and npm-build executions; add maven-test-skip profile that sets it true when -Dmaven.test.skip=true is passed, so release builds skip the frontend phase Documentation: - Add missing --crd-color-route, --crd-color-doTry, --crd-color-doCatch, and --crd-color-doFinally CSS custom properties to the theming section - Update build.mjs to auto-generate THIRD-PARTY-NOTICES.txt from the esbuild .LEGAL.txt sidecar so it stays in sync with bundled package versions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add node-type icons from Lucide (https://lucide.dev, ISC License) as inlined SVG path strings and update build.mjs to append their attribution to the esbuild-generated .LEGAL.txt sidecar and THIRD-PARTY-NOTICES.txt, since esbuild's legalComments scanner only detects npm-bundled packages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses both reviewer concerns (davsclaus, oscerd): - Rewrite camel-route-diagram.js as a zero-dependency vanilla Web Component (HTMLElement + attachShadow) inlining both the layout engine and the component. No Lit, no esbuild, no Node.js — the file is committed directly to resources. - Remove frontend-maven-plugin, package.json, package-lock.json, build.mjs, smoke-test.html, and the esbuild .LEGAL.txt sidecar. The skipFrontendBuild property and maven-test-skip profile are no longer needed. - Strip Lit BSD-3-Clause entries from THIRD-PARTY-NOTICES.txt; only the Lucide ISC attribution (icon paths remain inlined) is kept. - Port the 8 Vitest layout.test.js tests to RouteDiagramLayoutEngineTest.java, covering computeSubtreeWidth and assignPositions through layoutRoute(). - Add thirdPartyNoticesMentionsLucide() assertion to WebComponentBundleTest, fixing the discrepancy between the PR description and the actual test coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- RouteDiagramHelperTest: remove public final modifier, drop wrong Javadoc copied from production class, fix import order (java.* before org.*), remove unused RouteInfo import, replace weak size-only assertions with containsExactly/endsWith checks, and split into four focused test methods - Add two new tests pinning the wrapText tail-truncation behavior that changed during consolidation into RouteDiagramHelper: one for the "remaining fits on last line without ellipsis" path, one for truncation - camel-route-diagram.js: append route index to SVG marker ID so multiple routes in one shadow root no longer share duplicate IDs - camel-route-diagram.js: remove this.#data = null from HTTP and parse error paths so all error branches consistently preserve the last good data, matching the network-error catch block behavior Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
The xref:components:others:diagram.adoc target does not exist in the published components catalog yet (diagram.adoc was added to main after the last release), so the PR doc-validation Antora build cannot resolve it. Replace the xref with a plain-text reference to avoid the build failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
🧪 CI tested the following changed modules:
✅ POM dependency changes: targeted tests included Changed managed plugins: com.mycila:license-maven-plugin Modules affected by dependency changes (672)
All tested modules (674 modules)
|
…se orthogonal edge paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claude Code on behalf of Adriano Machado
Implements the
camel-route-diagramweb component (CAMEL-23636), a framework-agnostic, zero-dependency SVG diagram renderer for Apache Camel routes. It is distributed as a single self-contained JS file insidecamel-diagram, ready to be embedded in the developer console or any HTML page.What this PR adds:
camel-diagramMaven module — a newcomponents/camel-diagrammodule with no Node.js or npm dependency. The web component is committed directly tosrc/main/resources/META-INF/resources/camel/diagram/camel-route-diagram.js; no build step is required.camel-route-diagram.js) — a pure-JS port ofRouteDiagramLayoutEngine: depth-first traversal, branch-aware column assignment, collision avoidance for multi-branch EIPs (choice/multicast/circuitBreaker), and a post-pass that prevents straight-line edges from passing through intermediate nodes.camel-route-diagram.js) — a vanilla Web Component (HTMLElement+attachShadow, no Lit) implementing<camel-route-diagram src="…" refresh="…" filter="…">that polls the route-structure dev-console endpoint, lays out nodes with the engine above, and renders an SVG with:<svg>(Firefox compatibility).--crd-color-*,--crd-bg,--crd-fg, …) and automatic dark-mode viaprefers-color-scheme.AbortController-based fetch cancellation so overlapping polls never produce stale renders.THIRD-PARTY-NOTICES.txtattributes the Lucide ISC license for the inlined icon SVG paths.camel-diagram.adoccomponent page and an entry in the 4.21 upgrade guide.WebComponentBundleTestverifies the JS file is present in the JAR, is non-empty, registerscustomElements.define('camel-route-diagram', ...), and thatTHIRD-PARTY-NOTICES.txtmentions Lucide with ISC attribution.RouteDiagramLayoutEngineTestports the former Vitest layout suite to JUnit, coveringcomputeSubtreeWidthandassignPositionsthroughlayoutRoute().Addressing review feedback (davsclaus, oscerd):
frontend-maven-pluginhas been removed frompom.xml.-Dquickly, release profiles, and CI on non-x86 agents are unaffected.camel-route-diagram.js.LEGAL.txt(was an esbuild artefact) and the Lit BSD-3-Clause entries fromTHIRD-PARTY-NOTICES.txt.thirdPartyNoticesMentionsLucide()assertion toWebComponentBundleTest, fixing the discrepancy between the PR description and test coverage.RouteDiagramLayoutEngineTest.java.Renders:
Light theme
Dark Theme
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.