diff --git a/CLAUDE.md b/CLAUDE.md index a02b464f8bd..b0c74d77725 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,59 +1,44 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with -code in this repository. - -For guidance on **designing a new feature** (API shape, browser-API wrapping, -signals, lifecycle, nullability, DOM events, bootstrap flow, Javadoc -expectations), see [DESIGN_GUIDELINES.md](DESIGN_GUIDELINES.md). This file -covers the operational side: repo structure, build commands, test workflow, -coding conventions, and general coding rules that apply to all changes. +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. ## Repository Overview -Vaadin Flow is the Java framework of Vaadin Platform for building modern -web applications. This is a large, multi-module Maven project that combines +Vaadin Flow is the Java framework of Vaadin Platform for building modern web +applications. This is a large, multi-module Maven project that combines server-side Java components with modern frontend tooling (Vite, TypeScript, React support). -### Key Architecture Components - -**Core Server Framework (`flow-server/`)**: -- Component system with server-side state management (`StateNode`, `StateTree`) -- DOM abstraction layer (`Element`, `Node`) that syncs with client-side -- JavaScript execution bridge (`JacksonCodec`) for seamless - client-server communication -- Routing system (`Router`, `RouteConfiguration`) with navigation lifecycle -- Dependency injection and instantiation (`Instantiator`, `Lookup`) -- Frontend asset management and bundling - -**Client-Server Communication**: -- Uses Jackson for JSON serialization/deserialization between Java objects - and JavaScript -- `executeJs()` methods allow calling JavaScript from server with automatic - parameter serialization -- Return values from JavaScript can be automatically deserialized into - Java beans -- WebSocket-based push communication (`PushConnection`, - `AtmospherePushConnection`) - -**Frontend Build System**: -- Vite-based development mode with hot reload -- Production bundling with webpack plugins -- TypeScript support with generated type definitions -- React and Lit template support -- Theme system with CSS custom properties - -**Multi-Module Structure**: -- `flow-server`: Core server-side framework -- `flow-client`: Client-side TypeScript/JavaScript code -- `flow-data`: Data binding and validation -- `flow-router`: Navigation and routing -- `flow-html-components`: Basic HTML component wrappers -- `flow-devloop-daemon`: Daemon for the `vaadin-dev` dev loop -- `flow-tests/`: Extensive integration test suite +### Technologies + +- Java 21+, Maven +- Jakarta EE (not Java EE), Spring Boot 4 integration +- Jackson for client-server JSON serialization +- Vite for frontend builds, TypeScript for the client engine +- JUnit and Mockito for unit tests, Vaadin TestBench for integration tests + +### Key Modules + +- `flow-server`: core server-side framework (state tree, `Element`, routing, DI) +- `flow-client`: client-side TypeScript/JavaScript engine +- `flow-data`: data binding and validation +- `flow-html-components`: basic HTML component wrappers +- `flow-plugins`: Maven and Gradle build plugins +- `flow-devloop-daemon`: daemon for the `vaadin-dev` dev loop +- `flow-tests/`: integration test suite - `vaadin-spring`: Spring Framework integration +See `guidelines/repository.md` for the full module map and +`guidelines/architecture.md` for how the pieces fit together. + +## Guidelines & Conventions + +Always read `CONVENTIONS.md` in full when **authoring** or **reviewing** code, and before **committing** or **opening a pull request** — it is the canonical list of checkable conventions. + +Design and implementation guidelines live in `guidelines/`. Read the chapters mapped in `guidelines/overview.md` selectively for the topics your work touches. + +The commit message format and the expectations for opening a pull request are in the Commit & PR Hygiene section of `CONVENTIONS.md`. + ## Development Commands ### Building and Testing @@ -68,10 +53,10 @@ mvn clean install -DskipTests # Note: To run tests, omit -DskipTests entirely (not -DskipTests=false) # Build specific module -cd flow-server && mvn clean install +mvn clean install -pl flow-server -am # Run tests for specific module -cd flow-server && mvn test +mvn test -pl flow-server # Run specific test class mvn test -Dtest=JacksonCodecTest @@ -83,16 +68,16 @@ mvn test -Dtest=JacksonCodecTest#testComplexTypeSerialization mvn test -Dtest="*Codec*Test" # Run integration tests (automatically starts and stops server) -cd flow-tests/test-root-context && mvn verify +mvn verify -pl flow-tests/test-root-context # Run single integration test -mvn verify -Dit.test=ExecJavaScriptIT +mvn verify -pl flow-tests/test-root-context -Dit.test=ExecJavaScriptIT ``` ### Code Quality ```bash -# Format code +# Format code, must be run before every commit mvn spotless:apply # Check code formatting @@ -105,103 +90,7 @@ mvn checkstyle:check ### Frontend Development ```bash -# Frontend assets are managed by Maven plugins -# Vite dev mode is automatically started for development -# Manual frontend build (rare, usually automatic): +# Frontend assets are managed by Maven plugins, and Vite dev mode is started +# automatically during development. Manual frontend build (rare): cd flow-client && npm install && npm run build ``` - -## Coding Conventions - -- Use triple quotes (`"""`) for multi-line string blocks in Java text blocks. -- **When tests fail, code doesn't compile, or similar issues occur: Always - analyze why first. Do not start rewriting code.** -- **When writing code, names and comments should describe how the code works - and why, not what has changed from previous versions. Commit messages - capture change information, not the code itself.** -- **Always create proper tests for what should work first. If the tests - expose problems in the implementation, fix the implementation after the - tests have been created.** - -## Working with Key Components - -### JavaScript Execution and JSON Codec - -The `JacksonCodec` class handles serialization between Java and -JavaScript: - -- Parameters: Java objects → JSON → JavaScript variables (`$0`, `$1`, etc.) -- Return values: JavaScript objects → JSON → Java beans -- Special handling for `Element` instances (sent as DOM references) -- Support for arbitrary objects via Jackson serialization - -When calling `executeJs()`, always pass values as parameters (`$0`, `$1`, -...) — never concatenate them into the expression string. See -[DESIGN_GUIDELINES.md](DESIGN_GUIDELINES.md) for the full rules. - -### State Management - -Flow uses a tree-based state management system: -- `StateNode`: Represents component state on server -- `StateTree`: Manages entire application state tree -- `NodeFeature`: Different aspects of node state (properties, children, etc.) -- Changes are automatically synchronized to client - -### Component Development - -Components extend `Component` and use: -- `Element`: Low-level DOM manipulation -- Property synchronization via `@Synchronize` -- Event handling with `@DomEvent` -- Client-side callbacks with `@ClientCallable` - -### Testing - -**Unit Tests**: Located in `src/test/java/` in each module -- Use JUnit 4/5 -- Heavy use of Mockito for mocks -- Focus on individual class behavior - -**Integration Tests**: Located in `flow-tests/` -- Use TestBench for browser automation -- Test full client-server interaction -- Require running application server -- **When an IT fails: Use Playwright to debug the browser behavior and - understand what's actually happening in the UI.** - -## Common Patterns - -**Test Improvements**: When improving tests, focus on: -- Verifying actual behavior rather than just "not null" -- Testing JSON structure and content for serialization -- Adding comprehensive edge case coverage - -**JavaScript Integration**: When working with `executeJs()`: -- Remember Element parameters become DOM references or null -- Return values can be deserialized to Java beans automatically -- Use Jackson-compatible types for seamless serialization - -**Architecture Changes**: This is a complex, interconnected system: -- Changes to core classes like `StateNode` or `Element` have wide impact -- Frontend changes require corresponding server-side updates -- Always run relevant test suites after modifications - -## Important Notes - -- Java 21+ required for development -- Uses Jakarta EE (not Java EE) -- Spring Boot 4 integration available -- Hot reload available in development mode -- Extensive CI/CD pipeline with multiple test configurations -- When creating a commit that will resolve an issue in the same repository, - add "Fixes #issuenumber" to the commit message -- When creating a PR, mark it as a draft on GitHub and remind the user - about reviewing the code themselves and marking the PR ready -- Don't add `@since` to javadocs -- When adding unit tests, add only the essential ones and not more than that -- Use `test:` instead of `fix:` when fixing only tests - -See [DESIGN_GUIDELINES.md](DESIGN_GUIDELINES.md) for design-level guidance -(API shape, signals, sealed types, naming of components that wrap HTML -elements, DOM event naming, browser-wrapping conventions, supported -browsers, bootstrap data flow, Javadoc for wrapped browser APIs). diff --git a/CONVENTIONS.md b/CONVENTIONS.md new file mode 100644 index 00000000000..e5ae5c8f8b1 --- /dev/null +++ b/CONVENTIONS.md @@ -0,0 +1,180 @@ +# Conventions + +The canonical list of checkable conventions for this repository. Read it in +full when authoring or reviewing code. Design-level reasoning behind several of +these rules lives in `guidelines/` — see `guidelines/overview.md`. + +## Public API + +Find the existing precedent before designing new API. Flow already has patterns +for per-UI facades, reactive signals, sealed result hierarchies, Jackson wire +records and browser-API wrappers — match the existing shape instead of +inventing a new one. + +Expose observable state as a `Signal` rather than as a listener API, and +name the accessor with a `Signal` suffix (`localeSignal()`, +`availabilitySignal()`). + +Cache the read-only wrapper of a signal in a field instead of calling +`asReadonly()` per access. Every call allocates a fresh instance, so the +identity of the returned signal would otherwise be unstable. + +Seed a signal with a meaningful default rather than `null`. When the initial +state is "no data yet", use a sentinel enum constant (e.g. `UNKNOWN`) or a +dedicated record (e.g. `Pending`) so callers can pattern match without a +`case null` arm. + +Keep framework-only mutators off user-facing classes. The read surface belongs +on the user-facing class or facade, the write surface on `UIInternals` or an +equivalent internal-only class — a setter annotated "for framework use only" on +a class applications read from is a DX hazard. + +Make the constructor of a stateful handle that an API hands out (e.g. +`GeolocationWatcher`) package-private, so application code can not bypass the +entry point that creates it. + +Prefer an immutable record with a builder over a long parameter list for +tunable options, and validate in the compact constructor. + +Do not introduce an interface and an abstract class as a pair speculatively. +Ship a single class named `Xyz` unless you can demonstrate today a useful +implementation that does not extend the abstract class. See +`guidelines/design.md` for the full rule set. + +Use a sealed interface with record subtypes for values that are "one of N +things", and design for exhaustive `switch` expressions — do not add `default:` +arms over a sealed set. + +Tie resources that outlive a single request (watches, DOM listeners, timers, +client-side subscriptions) to a component's lifecycle by accepting a +`Component owner` and registering a `DetachListener`. Expose an explicit, +idempotent `stop()` for mid-view cancellation. + +Name a component that wraps an HTML element after the element itself, and add +the `Native` prefix only when the plain name is taken or when it invites a +mistake that goes unnoticed. Do not introduce further `Html…` names. See +`guidelines/design.md`. + +Renaming an existing public class is a breaking change. Add the new class, +deprecate the old one with a `@deprecated` pointer to the replacement, and +remove it in the next major. + +## Nullability + +Apply `@NullMarked` (JSpecify) at the package level and annotate only what +genuinely may be null with `@Nullable`. + +Prefer a sentinel value over a nullable return in the public API. Jackson wire +records are the legitimate exception, because the wire format permits +omissions — keep the wire record private and translate to a non-null public +shape at the boundary. + +Put `@Nullable` on the declared type (`ValueSignal<@Nullable X>`). NullAway +infers it for the constructor, so no repeated type argument or type witness is +needed. + +## Client-Side JavaScript + +Never concatenate values into an `executeJs` expression string. Pass them as +parameters and reference them positionally (`$0`, `$1`, …). + +Never build JSON by string concatenation. Use Jackson for construction. + +Log client-side `executeJs` errors at `DEBUG`, not at `WARN` or `ERROR`. A +failed JS call usually means the feature is unavailable, not that there is a +server bug. + +Put non-trivial JavaScript in its own file rather than inlining it in a Java +string, and prefer TypeScript for new files. See +`guidelines/browser-integration.md` for which of the two homes to use. + +Keep client-side global state and helper functions under `window.Vaadin.Flow`. + +Make `init(element)` installers idempotent. Track installations per element +with a `WeakMap` and dispose the previous listeners before attaching new ones. + +Prefix custom DOM events with `vaadin-` (e.g. `vaadin-geolocation-position`). + +Only write client code targeting the supported browsers listed in +`guidelines/browser-integration.md`. No fallbacks and no polyfills for anything +else. + +Probe for feature availability without calling the feature itself, since +calling it usually triggers a permission prompt. + +Update both sides in the same PR when a change touches the client-server +protocol or a DOM event contract. + +## Build & Dependencies + +Do not add a dev-runtime artifact as a `compile` or `runtime` scope dependency +of a build plugin module (`flow-plugins/flow-plugin-base`, +`flow-maven-plugin`, …). Plugin dependencies are resolved into the plugin +classloader before any goal runs, so they are downloaded by every build +including `-Pproduction`, they break offline and air-gapped builds, they show +up in SBOM audits, and every module that extends the plugin base inherits them. +When a goal needs a dev-only jar, resolve it from the project's own artifacts +at runtime and invoke it reflectively through a throwaway `URLClassLoader`. + +Do not use `provided` scope as a workaround for that: Maven only loads +`compile` and `runtime` dependencies into the plugin realm, so `provided` turns +the problem into a `NoClassDefFoundError` at goal execution time. + +Derive the version of a provisioned tool from the project's own dependency tree +instead of pinning it in the plugin, otherwise the pre-provisioned artifact +does not match what the running process expects and the network is hit anyway. + +Extract a shared utility instead of copying a class or method between modules. +When two modules need the same logic, move it to the module they both depend +on. + +## Javadoc + +Do not add `@since` tags. What to write in Javadoc, and how to document a +wrapped browser API, is covered by `guidelines/documenting.md`. + +## Testing + +Write the tests that should pass first. If they expose problems in the +implementation, fix the implementation — do not rewrite the tests to match a +broken implementation. + +Analyze why a test fails, code does not compile, or a build breaks, before +changing anything. Do not start rewriting code. + +Keep the unit test count minimal — add only the essential cases. + +Assert concrete outputs, not just "not null". Verify JSON structure and content +for serialization, and cover the edge cases that the change actually +introduces. + +Add an integration test view under `flow-tests/test-root-context/` for +browser-facing features, and exercise both the happy path and the error branch. + +Debug a failing integration test with Playwright before guessing. Look at what +the browser is actually doing. + +## Code Style + +Run `mvn spotless:apply` before every commit. + +Names and comments describe how the code works and why, not what changed from a +previous version. + +Use Java text blocks for multi-line strings instead of string concatenation. + +## Commit & PR Hygiene + +Prefix commit messages with the type: `feat:`, `fix:`, `test:`, `refactor:`, +`chore:`, `docs:`, `ci:`. Use `test:` when the change only touches tests, and +add `!` after the prefix for a breaking change (`chore!:`). + +Add the affected module or area as a scope when it narrows the message +usefully — `fix(flow-client):`, `feat(hilla):`, `chore(deps):`. The scope is +optional; a bare `fix:` is fine for changes that span modules. + +Add `Fixes #issuenumber` to the commit message when the commit resolves an +issue in this repository. + +Open pull requests as drafts, and remind the author to self-review before +marking them ready. diff --git a/guidelines/architecture.md b/guidelines/architecture.md new file mode 100644 index 00000000000..4b64011760b --- /dev/null +++ b/guidelines/architecture.md @@ -0,0 +1,76 @@ +# Architecture + +How the core pieces of Flow fit together, and what to keep in mind when +touching them. + +## State management + +Flow keeps the server-side representation of the UI in a tree and synchronizes +changes to the client automatically: + +- `StateNode` — the state of a single component or element on the server. +- `StateTree` — the whole application state tree for one UI. +- `NodeFeature` — an aspect of a node's state (properties, children, listeners, + …). A node carries only the features it needs. + +Changes made to the tree during a request are collected and sent to the client +at the end of the request. Nothing needs to be pushed manually. + +## DOM abstraction + +`Element` and `Node` are the server-side mirror of the browser DOM. Components +build on top of them: + +- `Element` for low-level DOM manipulation. +- `@Synchronize` to synchronize a client-side property back to the server. +- `@DomEvent` to map a DOM event to a server-side component event. +- `@ClientCallable` to expose a server-side method to client-side code. + +An element's `nodeId` is not a stable identifier — it is `-1` until the node is +attached. Use an attachment-independent identifier (e.g. a UUID) when you need +one. + +## Client-server communication + +`JacksonCodec` handles serialization between Java and JavaScript: + +- Parameters: Java objects → JSON → JavaScript variables (`$0`, `$1`, …). +- Return values: JavaScript objects → JSON → Java records or beans. +- `Element` instances are sent as DOM references (or `null`). +- Arbitrary objects are supported via Jackson serialization. + +Always pass values to `executeJs()` as parameters, never concatenated into the +expression string. See [Browser Integration](browser-integration.md) for the +full rules on calling into the browser. + +Push uses a WebSocket-based connection (`PushConnection`, +`AtmospherePushConnection`). + +## Routing + +`Router` and `RouteConfiguration` implement navigation, with a lifecycle of +before-leave, before-enter and after-navigation observers. Route resolution is +driven by `@Route` annotations, discovered at startup or registered +programmatically. + +## Instantiation and lookup + +`Instantiator` creates the objects Flow needs (views, listeners, converters) so +that a DI container can take over. `Lookup` resolves the SPI implementations +available in the current environment. Prefer going through these rather than +constructing implementations directly. + +## Frontend build + +- Vite-based development mode with hot reload. +- Production bundling, with a pre-built default bundle when the application + does not need a custom one. +- TypeScript support with generated type definitions. +- Lit and React template support. +- Theme system built on CSS custom properties. + +## Accessing the UI + +Prefer reaching the UI through the component (`getUI()`) or through an event +that provides it (e.g. `AttachEvent`). `UI.getCurrent()` is a last resort for +code that has no component instance available. diff --git a/guidelines/browser-integration.md b/guidelines/browser-integration.md new file mode 100644 index 00000000000..cfe97dbafb9 --- /dev/null +++ b/guidelines/browser-integration.md @@ -0,0 +1,139 @@ +# Browser Integration + +How to wrap a browser or JavaScript API in Java, where the client-side code +lives, and how the two sides talk to each other. + +For the Javadoc expectations that come with a browser-API wrapper, see +[Documenting](documenting.md). + +## Supported browsers + +Only write client code targeting these. **No fallbacks, no polyfills** for +anything else: + +- Chrome (evergreen) +- Firefox (evergreen) +- Firefox Extended Support Release (ESR) +- Safari 17 or newer (latest minor version in each major series) +- Edge (Chromium, evergreen) + +## JavaScript location and globals + +- **Non-trivial JS goes in its own file**, never inlined beyond a + one-liner in a Java string. Two valid homes: + - `flow-client/src/main/frontend/Xxx.ts` imported from `Flow.ts` + (`import './Xxx';`). Use this for platform-level features that + need to be available before the bootstrap handshake (anything + referenced from `collectBrowserDetails`, anything that must attach + `document` / `window` listeners before the first user interaction). + Precedents: `Geolocation.ts`, `PageVisibility.ts`. Prefer TypeScript + here — new files should not be `.js`. + - `META-INF/frontend/xxx.js` loaded via `@JsModule("./xxx.js")` on + `UI.java` or a component. Use this when the script is tied to a + specific Java API surface and does not need to run at bootstrap + time. +- **Global state and helper functions live under `window.Vaadin.Flow`** + (e.g. `window.Vaadin.Flow.geolocation`, + `window.Vaadin.Flow.pageVisibility`, + `window.Vaadin.Flow.componentSizeObserver`). Use annotations on + `UI.java` for scripts that need to run globally. +- **`init(element)` installers must be idempotent.** A facade may call + `window.Vaadin.Flow.xxx.init(this)` more than once per UI element + (lazy (re)arming from a signal accessor, navigation to a view that + re-subscribes, etc.). Track installations per element (WeakMap) and + dispose the previous set of listeners before attaching new ones so + the element never carries duplicates. + +## `executeJs` parameter passing + +- **Never** concatenate values into the expression string. Always pass + them as parameters and reference them positionally (`$0`, `$1`, ...). + String concatenation is a prompt for injection bugs and quoting + nightmares. +- **Never build JSON manually by string concatenation.** Use Jackson 3 for + construction. +- Element parameters arrive on the client as DOM references (or `null`); + plan for that on both sides. +- Return values from JS can be deserialised to Java records automatically; + use a private record for the wire shape. +- **Log `executeJs` client-side errors at `DEBUG`, not WARN/ERROR.** A + failed JS call usually means the feature is unavailable (user denied + permission, API missing, insecure context) — not a server bug. The + pattern is `.then(ok -> {}, err -> LOGGER.debug("X failed: {}", err))`. + +## DOM event naming + +- **Prefix custom DOM events with `vaadin-`** — e.g. + `vaadin-geolocation-position`, not `geolocation-position`. This keeps + the event namespace distinct and grepable. +- Event payloads travel as Jackson-annotated records. Keep the wire shape + faithful to what the browser produces (e.g. `long timestamp` not + `Instant`) and provide convenience accessors on the public type. + +## Server ↔ client signalling patterns + +For streaming and state-change wiring, keep DOM events as **transport** +and `Signal` as **state**. Applications should subscribe to the signal; +the DOM events are an implementation detail of the facade. + +- **Event-to-Signal bridging.** The client dispatches a + `vaadin-xxx-position` / `vaadin-xxx-error` CustomEvent per update; the + server-side facade has a DOM listener that pulls the detail record + and writes it to the private `ValueSignal`. Applications subscribe to + the signal. +- **Client-initiated state-change bridge-back.** For state that changes + without a server-initiated request (permission change, network + online/offline, window resize), the client dispatches a + `vaadin-xxx-change` event on `document.body` (which is the UI's root + element on the server). The facade constructor registers a listener + on `ui.getElement()` and forwards the detail into the same + `UIInternals` signal the bootstrap path seeds. No polling required. +- **Stable client-side keys for async browser handles.** When the + browser API returns an opaque id asynchronously (e.g. + `watchPosition()`), don't try to round-trip it back to the server to + later cancel. Pre-generate a UUID on the server, pass it as an + `executeJs` parameter, and have the client's wrapper store its own + `Map`. Both sides then use the same key for + subsequent operations (`clearWatch(key)` on the client looks up the + browser-assigned id). + +## Bootstrap-time data + +If a feature needs an initial value before the first user interaction, +thread it through the bootstrap handshake rather than waiting for a +round-trip: + +- Client collects the value in `collectBrowserDetails` (make that + function async if needed) and appends it to the init request as a + `v-xxx` parameter. The TS that produces the value must be imported + from `Flow.ts` so it is loaded when `collectBrowserDetails` runs — + `@JsModule` on `UI.java` loads too late for this path. +- Server reads it in `ExtendedClientDetails.fromJson` and seeds the + appropriate `UIInternals` field / signal. +- The public Java signal picks up the value on UI attach — no + additional round-trip required. +- Seed the server-side signal with a sentinel (`UNKNOWN`, `Pending`, …) + so the brief window between attach and handshake completion is + distinguishable from a genuine reading. Precedents: + `GeolocationAvailability.UNKNOWN`. + +## Feature-capability detection + +Probe for feature availability **without calling the feature itself** +— calling it usually triggers a permission prompt, which defeats the +point of probing. Useful primitives: + +- `window.isSecureContext` — HTTPS or `localhost`. Most sensitive + browser APIs require this. +- `document.featurePolicy?.allowsFeature("xxx")` — Chromium-only; + Firefox and Safari don't expose a feature-policy introspection API. + Absence of the API should be treated as "allowed", not "unsupported". +- `navigator.permissions.query({ name: "xxx" })` — returns a + `PermissionStatus` whose `.state` is `"granted" | "denied" | + "prompt"` and which also emits a `change` event. Safari may reject + with a TypeError for specific permission names; catch and fall back + to an `UNKNOWN` sentinel. +- Expose the result to the server via the bootstrap param pattern + above, plus a `vaadin-xxx-availability-change` event for subsequent + changes. + diff --git a/DESIGN_GUIDELINES.md b/guidelines/design.md similarity index 54% rename from DESIGN_GUIDELINES.md rename to guidelines/design.md index 658cef85abf..948ccca7002 100644 --- a/DESIGN_GUIDELINES.md +++ b/guidelines/design.md @@ -1,12 +1,17 @@ -# DESIGN_GUIDELINES.md +# Design -Guidance for designing new features in Vaadin Flow. Consult this before -starting any non-trivial new API — especially anything that wraps a browser -or JavaScript API, exposes observable state, or manages a resource with a -lifecycle. +Guidance for the shape of new public Java API in Vaadin Flow. Consult this +before starting any non-trivial new API — especially anything that wraps a +browser or JavaScript API, exposes observable state, or manages a resource with +a lifecycle. -For day-to-day commands, repo structure, and testing workflow, see -[CLAUDE.md](CLAUDE.md). +This chapter covers the Java surface only. The rest of the material for a +browser-API wrapper lives in sibling chapters: + +- [Browser Integration](browser-integration.md) — where the client-side code + lives, `executeJs`, DOM event naming, bootstrap data, capability detection. +- [Documenting](documenting.md) — what the Javadoc of a wrapper has to explain. +- [Testing](testing.md) — mocking the browser API in an integration test. ## Before you start @@ -15,11 +20,10 @@ For day-to-day commands, repo structure, and testing workflow, see browser-API wrappers. Look at `Page`, `History`, `ExtendedClientDetails`, `VaadinSession.localeSignal`, `window.Vaadin.Flow.*` helpers, etc. — match the existing shape rather than inventing a new one. -- **Understand the blast radius.** Changes to `StateNode`, `Element`, or the - codec layer ripple across the codebase. If your design needs to touch - them, plan for full test runs and a longer review cycle. -- **Frontend ↔ server is a contract.** Any protocol or DOM-event change - needs both sides updated in the same PR. +- **Understand the blast radius.** See [Repository](repository.md) — changes to + `StateNode`, `Element` or the codec layer ripple across the codebase. +- **Frontend ↔ server is a contract.** Any protocol or DOM-event change needs + both sides updated in the same PR. ## API shape @@ -231,191 +235,3 @@ Decide it from the rules below rather than case by case. explaining what to use instead, and remove it in the next major — the way `Label` was replaced by `NativeLabel` in 24.1 and removed in 25. -## Browser / JavaScript integration - -### Wrap thin; document thick - -Java API wrapping a browser/JS API must be written for Java developers who -do not know the underlying JS API. Explain: what the method does in Java -terms; when to call it; what the parameters and return value mean; -threading and lifecycle expectations; any browser-specific caveats -(e.g. "Safari always returns UNKNOWN"). Do not assume the reader will -read the W3C spec or the `.ts` source. - -### Supported browsers - -Only write client code targeting these. **No fallbacks, no polyfills** for -anything else: - -- Chrome (evergreen) -- Firefox (evergreen) -- Firefox Extended Support Release (ESR) -- Safari 17 or newer (latest minor version in each major series) -- Edge (Chromium, evergreen) - -### JavaScript location and globals - -- **Non-trivial JS goes in its own file**, never inlined beyond a - one-liner in a Java string. Two valid homes: - - `flow-client/src/main/frontend/Xxx.ts` imported from `Flow.ts` - (`import './Xxx';`). Use this for platform-level features that - need to be available before the bootstrap handshake (anything - referenced from `collectBrowserDetails`, anything that must attach - `document` / `window` listeners before the first user interaction). - Precedents: `Geolocation.ts`, `PageVisibility.ts`. Prefer TypeScript - here — new files should not be `.js`. - - `META-INF/frontend/xxx.js` loaded via `@JsModule("./xxx.js")` on - `UI.java` or a component. Use this when the script is tied to a - specific Java API surface and does not need to run at bootstrap - time. -- **Global state and helper functions live under `window.Vaadin.Flow`** - (e.g. `window.Vaadin.Flow.geolocation`, - `window.Vaadin.Flow.pageVisibility`, - `window.Vaadin.Flow.componentSizeObserver`). Use annotations on - `UI.java` for scripts that need to run globally. -- **`init(element)` installers must be idempotent.** A facade may call - `window.Vaadin.Flow.xxx.init(this)` more than once per UI element - (lazy (re)arming from a signal accessor, navigation to a view that - re-subscribes, etc.). Track installations per element (WeakMap) and - dispose the previous set of listeners before attaching new ones so - the element never carries duplicates. - -### `executeJs` parameter passing - -- **Never** concatenate values into the expression string. Always pass - them as parameters and reference them positionally (`$0`, `$1`, ...). - String concatenation is a prompt for injection bugs and quoting - nightmares. -- **Never build JSON manually by string concatenation.** Use Jackson 3 for - construction. -- Element parameters arrive on the client as DOM references (or `null`); - plan for that on both sides. -- Return values from JS can be deserialised to Java records automatically; - use a private record for the wire shape. -- **Log `executeJs` client-side errors at `DEBUG`, not WARN/ERROR.** A - failed JS call usually means the feature is unavailable (user denied - permission, API missing, insecure context) — not a server bug. The - pattern is `.then(ok -> {}, err -> LOGGER.debug("X failed: {}", err))`. - -### DOM event naming - -- **Prefix custom DOM events with `vaadin-`** — e.g. - `vaadin-geolocation-position`, not `geolocation-position`. This keeps - the event namespace distinct and grepable. -- Event payloads travel as Jackson-annotated records. Keep the wire shape - faithful to what the browser produces (e.g. `long timestamp` not - `Instant`) and provide convenience accessors on the public type. - -### Server ↔ client signalling patterns - -For streaming and state-change wiring, keep DOM events as **transport** -and `Signal` as **state**. Applications should subscribe to the signal; -the DOM events are an implementation detail of the facade. - -- **Event-to-Signal bridging.** The client dispatches a - `vaadin-xxx-position` / `vaadin-xxx-error` CustomEvent per update; the - server-side facade has a DOM listener that pulls the detail record - and writes it to the private `ValueSignal`. Applications subscribe to - the signal. -- **Client-initiated state-change bridge-back.** For state that changes - without a server-initiated request (permission change, network - online/offline, window resize), the client dispatches a - `vaadin-xxx-change` event on `document.body` (which is the UI's root - element on the server). The facade constructor registers a listener - on `ui.getElement()` and forwards the detail into the same - `UIInternals` signal the bootstrap path seeds. No polling required. -- **Stable client-side keys for async browser handles.** When the - browser API returns an opaque id asynchronously (e.g. - `watchPosition()`), don't try to round-trip it back to the server to - later cancel. Pre-generate a UUID on the server, pass it as an - `executeJs` parameter, and have the client's wrapper store its own - `Map`. Both sides then use the same key for - subsequent operations (`clearWatch(key)` on the client looks up the - browser-assigned id). - -### Bootstrap-time data - -If a feature needs an initial value before the first user interaction, -thread it through the bootstrap handshake rather than waiting for a -round-trip: - -- Client collects the value in `collectBrowserDetails` (make that - function async if needed) and appends it to the init request as a - `v-xxx` parameter. The TS that produces the value must be imported - from `Flow.ts` so it is loaded when `collectBrowserDetails` runs — - `@JsModule` on `UI.java` loads too late for this path. -- Server reads it in `ExtendedClientDetails.fromJson` and seeds the - appropriate `UIInternals` field / signal. -- The public Java signal picks up the value on UI attach — no - additional round-trip required. -- Seed the server-side signal with a sentinel (`UNKNOWN`, `Pending`, …) - so the brief window between attach and handshake completion is - distinguishable from a genuine reading. Precedents: - `GeolocationAvailability.UNKNOWN`. - -### Feature-capability detection - -Probe for feature availability **without calling the feature itself** -— calling it usually triggers a permission prompt, which defeats the -point of probing. Useful primitives: - -- `window.isSecureContext` — HTTPS or `localhost`. Most sensitive - browser APIs require this. -- `document.featurePolicy?.allowsFeature("xxx")` — Chromium-only; - Firefox and Safari don't expose a feature-policy introspection API. - Absence of the API should be treated as "allowed", not "unsupported". -- `navigator.permissions.query({ name: "xxx" })` — returns a - `PermissionStatus` whose `.state` is `"granted" | "denied" | - "prompt"` and which also emits a `change` event. Safari may reject - with a TypeError for specific permission names; catch and fall back - to an `UNKNOWN` sentinel. -- Expose the result to the server via the bootstrap param pattern - above, plus a `vaadin-xxx-availability-change` event for subsequent - changes. - -## Documentation - -- Javadoc for public API explains the *why* and the caveats, not just the - type signature. -- Call out reliability concerns prominently — if a value is best-effort, - say so, and enumerate the browsers where it degrades. -- Prefer short, runnable examples in the class-level Javadoc. Keep - examples consistent with the real platform APIs (e.g. if you show - `map.setCenter(...)`, use the real Vaadin Map `Coordinate(longitude, - latitude)` shape). -- **Do not add `@since` tags.** -- Javadoc describes the code today, not what changed. Change history - belongs in commit messages. - -## Testing new features - -- **Write the tests that should pass first.** If they expose problems in - the implementation, fix the implementation afterwards — don't rewrite - the tests to match a broken implementation. -- Keep the unit test count minimal — only the essential cases. More tests - are not better; focused tests are. -- For browser-facing features, add an IT view under - `flow-tests/test-root-context/` that mocks the relevant browser API - and exercises both happy-path and error branches. Use an option - value to trigger errors deterministically (for geolocation, - `maximumAge === -1` works). If the API streams updates (e.g. - `watchPosition`), use `setInterval` to simulate them and verify - that the matching cancel call (e.g. `clearWatch`) actually stops - them. -- ITs assert concrete outputs, not just "not null". If floating-point - arithmetic would make assertions brittle, simplify the mock to emit - stable values (different timestamps suffice for uniqueness). -- Add a short settle pause before snapshotting counts after a - stop-like action — an in-flight event can still land right after the - stop marker appears in the DOM. -- When an IT fails, debug with Playwright before guessing — see what the - browser is actually doing. - -## Commit / PR hygiene - -- When a commit resolves an issue in this repo, add `Fixes #issuenumber` - to the message. -- Use `test:` prefix for commits that only touch tests; `fix:` is for - production-code fixes; `feat:` is for new features. -- When opening a PR, mark it as draft. Remind the author to self-review - before marking it ready. diff --git a/guidelines/documenting.md b/guidelines/documenting.md new file mode 100644 index 00000000000..476d23221ab --- /dev/null +++ b/guidelines/documenting.md @@ -0,0 +1,26 @@ +# Documenting + +## Wrap thin; document thick + +Java API wrapping a browser/JS API must be written for Java developers who +do not know the underlying JS API. Explain: what the method does in Java +terms; when to call it; what the parameters and return value mean; +threading and lifecycle expectations; any browser-specific caveats +(e.g. "Safari always returns UNKNOWN"). Do not assume the reader will +read the W3C spec or the `.ts` source. + +## Javadoc + +- Javadoc for public API explains the *why* and the caveats, not just the + type signature. +- Call out reliability concerns prominently — if a value is best-effort, + say so, and enumerate the browsers where it degrades. +- Prefer short, runnable examples in the class-level Javadoc. Keep + examples consistent with the real platform APIs (e.g. if you show + `map.setCenter(...)`, use the real Vaadin Map `Coordinate(longitude, + latitude)` shape). +- Javadoc describes the code today, not what changed. Change history + belongs in commit messages. + +The mechanical Javadoc rules — such as not adding `@since` tags — are listed +in [`CONVENTIONS.md`](../CONVENTIONS.md). diff --git a/guidelines/overview.md b/guidelines/overview.md new file mode 100644 index 00000000000..21e21571589 --- /dev/null +++ b/guidelines/overview.md @@ -0,0 +1,23 @@ +# Flow Guidelines + +These guidelines describe how features in Vaadin Flow — the Java server-side +framework of Vaadin Platform — should be designed and implemented in the `flow` +repository. Chapters can be read selectively for the topics your work touches. + +Treat these as guidelines, not hard rules. They are best practices that should +be followed by default, but can be deviated from when necessary to make +something work. + +For the canonical list of checkable conventions see [`CONVENTIONS.md`](../CONVENTIONS.md). +For repository-level commands (build, test, format) see [`CLAUDE.md`](../CLAUDE.md). + +## Chapters + +| Chapter | Topic | +| ------------------------------------------- | ------------------------------------------------------------------------------ | +| [Repository](repository.md) | Tech stack, module layout, where things live, Maven and build plugins. | +| [Architecture](architecture.md) | State tree, `Element`, the Jackson codec, routing, component development. | +| [Design](design.md) | Shape of new public Java API: facades, signals, sealed types, naming, lifecycle. | +| [Browser Integration](browser-integration.md) | Wrapping browser APIs, `executeJs`, DOM events, bootstrap data, capability detection. | +| [Documenting](documenting.md) | Javadoc expectations, documenting wrapped browser APIs. | +| [Testing](testing.md) | Unit tests, integration tests, debugging failures. | diff --git a/guidelines/repository.md b/guidelines/repository.md new file mode 100644 index 00000000000..f8eb2498488 --- /dev/null +++ b/guidelines/repository.md @@ -0,0 +1,66 @@ +# Repository + +## Technology stack + +- **Java 21+**, **Maven** (large multi-module project). +- **Jakarta EE** (not Java EE). Spring Boot 4 integration is available. +- **Jackson** for JSON serialization between Java objects and JavaScript. +- **Vite** for development mode with hot reload, and for production bundling. +- **TypeScript** for the client engine and for generated type definitions. +- **JUnit** and **Mockito** for unit tests, **Vaadin TestBench** for browser + integration tests. + +## Module structure + +Every top-level Maven module of the repository: + +| Module | Contents | +| -------------------------------- | ------------------------------------------------------------------------------ | +| `flow-server` | Core server-side framework: state tree, DOM abstraction, routing, DI, frontend asset management. | +| `flow-client` | Client-side TypeScript/JavaScript engine. | +| `flow-data` | Data binding and validation. | +| `flow-html-components` | Basic HTML component wrappers (`Div`, `Anchor`, `NativeLabel`, …). | +| `flow-html-components-testbench` | TestBench elements for the HTML components. | +| `flow-react` | React integration. | +| `flow-push` | WebSocket-based push. | +| `flow-webpush` | Web Push notifications. | +| `flow-dnd` | Drag and drop. | +| `flow-lit-template` | Lit template support. | +| `flow-polymer-template` | Polymer template support (legacy). | +| `flow-polymer2lit` | Polymer to Lit converter. | +| `flow-plugins` | Build plugins: `flow-plugin-base`, `flow-maven-plugin`, `flow-gradle-plugin`, `flow-dev-bundle-plugin`. | +| `flow-build-tools` | Frontend build tooling shared by the plugins and the dev server. | +| `flow-devloop-daemon` | Daemon for the `vaadin-dev` dev loop. | +| `vaadin-dev-server` | Development tooling served to the browser. | +| `vaadin-spring` | Spring Framework integration. | +| `flow-server-production-mode` | Wrapper artifact whose `web-fragment.xml` turns on production mode. | +| `flow-jandex` | Jandex index of the Flow packages, for use outside Vaadin Platform. | +| `flow` | Aggregate POM that pulls in the modules an application needs. | +| `flow-bom` | Bill of materials. | +| `flow-test-util` | Test utilities (TestBench base classes, IT helpers). | +| `flow-test-generic` | Generic test utilities shared by the modules. | +| `flow-tests/` | Integration test suite. | + +Routing lives in `flow-server` — there is no separate router module. + +## Build plugins + +`flow-plugins` holds the build-time tooling. Anything declared as a dependency +of a plugin module is loaded into the Maven plugin classloader before any goal +runs — in every build, including production ones. Keep dev-runtime artifacts +out of that dependency set and resolve them from the project's own artifacts at +goal execution time instead. See the Build & Dependencies section of +[`CONVENTIONS.md`](../CONVENTIONS.md). + +## Code style + +Formatting is applied by `mvn spotless:apply` and validated by +`mvn spotless:check` and `mvn checkstyle:check`. Run the formatter before every +commit — the CI validation job fails on unformatted code. + +## Blast radius + +This is a complex, interconnected system. Changes to core classes such as +`StateNode`, `Element` or the codec layer ripple across the codebase — plan for +full test runs and a longer review cycle. Frontend changes generally require +corresponding server-side changes in the same PR, and vice versa. diff --git a/guidelines/testing.md b/guidelines/testing.md new file mode 100644 index 00000000000..07c3ad4de5b --- /dev/null +++ b/guidelines/testing.md @@ -0,0 +1,42 @@ +# Testing + +## Where tests live + +**Unit tests** live in `src/test/java/` in each module. They use JUnit and make +heavy use of Mockito, and they focus on the behavior of individual classes. + +**Integration tests** live in `flow-tests/`. They use TestBench for browser +automation, exercise the full client-server interaction, and require a running +application server (the Maven build starts and stops it). + +## Writing tests + +- **Write the tests that should pass first.** If they expose problems in + the implementation, fix the implementation afterwards — don't rewrite + the tests to match a broken implementation. +- Keep the unit test count minimal — only the essential cases. More tests + are not better; focused tests are. +- For browser-facing features, add an IT view under + `flow-tests/test-root-context/` that mocks the relevant browser API + and exercises both happy-path and error branches. Use an option + value to trigger errors deterministically (for geolocation, + `maximumAge === -1` works). If the API streams updates (e.g. + `watchPosition`), use `setInterval` to simulate them and verify + that the matching cancel call (e.g. `clearWatch`) actually stops + them. +- ITs assert concrete outputs, not just "not null". If floating-point + arithmetic would make assertions brittle, simplify the mock to emit + stable values (different timestamps suffice for uniqueness). +- Add a short settle pause before snapshotting counts after a + stop-like action — an in-flight event can still land right after the + stop marker appears in the DOM. +- When improving existing tests, verify actual behavior rather than just + "not null" — assert the JSON structure and content for serialization, and + cover the edge cases the change actually introduces. + +## Debugging failures + +- Analyze *why* a test fails, code does not compile, or a build breaks, before + changing anything. Do not start rewriting code. +- When an integration test fails, use Playwright to see what the browser is + actually doing in the UI, rather than guessing at the cause.