no transitive deps#79
Draft
mstahv wants to merge 6 commits into
Draft
Conversation
Phase 1 of the de-Kotlin effort on the shared module: the 12 Kotlin files
under shared/src/main/kotlin/com/vaadin/browserless/mocks/ (servlet + Vaadin
mocks: MockContext, MockRequest, MockResponse, MockHttpSession, MockedUI,
MockService, MockVaadinServlet, MockVaadinHelper, MockVaadinSession,
MockInstantiator, MockHttpEnvironment, SessionAttributeMap) become 14 Java
files under shared/src/main/java/com/vaadin/browserless/mocks/. No behavior
change; this is a mechanical translation.
Structural notes:
- MockHttpEnvironment.kt held three things (the singleton, the
MockServletConfig class, an extension fn). It splits in Java into
MockHttpEnvironment + MockServletConfig + MockUtils (housing
putOrRemove for the three remaining call sites).
- MockVaadinServlet's top-level extension helpers (serviceSafe,
createVaadinServletRequest/Response, _createVaadinSession, WebBrowser
factory) become public static methods on MockVaadinServlet. The
WebBrowser factory is renamed createWebBrowser to avoid clashing with
com.vaadin.flow.server.WebBrowser in Java syntax.
- SessionAttributeMap's `HttpSession.attributes` extension property is
gone; callers use `new SessionAttributeMap(session)` directly.
- MockInstantiator's `Instantiator by delegate` is replaced with
explicit forwarding of all 10 Instantiator methods to a held
delegate. The dead `ByteBuddyUtils` (referenced only by commented-out
code) is dropped.
- MockRequest keeps an explicit setUserInRole(BiPredicate) setter so
spring's MockSpringServlet keeps its current call sites — Kotlin's
`var isUserInRole` previously generated this setter on the bytecode.
Callers updated:
- shared/internal/MockVaadin.kt — uses MockVaadinServlet.* statics for
the 5 helpers that used to be top-level Kotlin fns.
- junit6/test/mocks/SessionAttributeMapTest.kt — instantiates
SessionAttributeMap directly.
- junit6/test/mocks/MockRequestTest.kt — wraps the role-checker lambda
in BiPredicate{} since the field type is no longer a Kotlin functional
type that SAM-converts implicitly at field-assignment.
- shared/test/mocks/BrowserlessLookupInitializerTest.kt — adjusted to
the new Map<Class<?>, Class<?>> field type on BrowserlessLookupInitializer
(the Kotlin original used Map<KClass<*>, KClass<*>>).
Runtime impact:
- kotlin-reflect (~3.3 MB) and kotlin-stdlib (~1.7 MB) are still on the
classpath for this branch — Phase 1 only ports the mocks/ package and
is not itself enough to drop the Kotlin runtime jars (PrettyPrintTree,
Grid, Locator, etc. still live in kotlin/).
- LOC delta: 12 .kt deleted (~1,529 LOC), 14 .java added (~2,354 LOC).
The expansion is mostly Java boilerplate: explicit getters/setters
around what were Kotlin `val`s, explicit Instantiator delegation, and
explicit try/catch around ReflectiveOperationException where Kotlin
silently propagated.
Verified by all four test suites:
- shared: 20 tests
- junit6: 1007 tests
- spring: 29 tests
- quarkus: 27 tests
All green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-phase + overview + self-review markdown describing the 5-phase
de-Kotlin effort on shared/src/main/. Intended as PR review context;
can be removed in a follow-up after all phases merge, or kept as
historical record of design decisions made during the port.
- overview.md — branch stack, merge order, runtime classpath
chart, translation patterns, footguns
- phase-1-mocks.md — mocks/ package port
- phase-2-internal-utils.md
— BasicUtils/ComponentUtils/etc. port
- phase-3-kotlin-reflect-drop.md
— PrettyPrintTree + Routes + reflect drop
- phase-4-locator-mockvaadin.md
— Locator engine + MockVaadin port
- phase-5-grid-kotlin-removal.md
— Grid port + kotlin-stdlib drop
- self-review.md — compact API-impact / Kotlin-user / test-
coverage assessment across all 5 phases
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 of de-Kotlin on the shared module: 8 utility files in
`shared/src/main/kotlin/com/vaadin/browserless/internal/` become 9 Java
files in `shared/src/main/java/com/vaadin/browserless/internal/`:
- BasicUtils, ComponentUtils, ElementUtils, DepthFirstTreeIterator,
Renderers, Shortcuts, TestingLifecycleHook, Utils
- plus a new TestingLifecycleHooks holder class for the mutable
`current` field + `cleanupDialogs()`, since Java interfaces can't
hold mutable static state the way a Kotlin top-level `var` can.
The ports are mechanical translation only — no behavior changes.
Kotlin extension functions and extension properties become public
static methods on the matching utility class (extension receiver
becomes the first parameter). Getter+setter pairs are emitted for
extension properties with both `get()` and `set()` blocks. The
Karibu-Testing leading-underscore convention (`_fireEvent`, `_text`,
`_isVisible`, `_saneFetchLimit`, etc.) is preserved.
Cross-Kotlin call-site updates in still-Kotlin files:
- Locator.kt, MockVaadin.kt, PrettyPrintTree.kt, Routes.kt — all
now call the new Java statics rather than relying on Kotlin
extension resolution. Locator/PrettyPrintTree have the largest
delta since they consume the most utility surface.
- component/Grid.kt — ~12 call sites rewritten to static-call form.
- 11 Kotlin test files — ~200+ extension call-sites switched to
static-call form.
Cross-Java call-site updates: 12 sites in shared/, spring/, and
quarkus/ — all that were importing `XxxKt` classes for the
bytecode-emitted Kotlin top-level statics. Now import the new Java
utility classes directly.
Deferred to Phase 4 (alongside SearchSpec): one Kotlin file
`internal/Matches.kt` (24 LOC) holds the `Component.matches(spec)`
extension that uses the SearchSpec DSL block, plus a tiny
`IntRange.size` extension still used by `component/Grid._dump` — both
will be ported when the Locator engine itself is.
Dead code dropped during the port: `serializeToBytes`, `deserialize`,
`serializeDeserialize` — three top-level functions in Utils.kt with
zero callers anywhere.
Adjustments worth flagging beyond the brief:
- `Component.dataProvider`'s `HasDataProvider<*>` branch was dropped
— the original Kotlin's `is HasDataProvider<*> -> this.dataProvider`
relied on a recursive extension lookup that no longer resolves in
current Vaadin (the interface only exposes `setDataProvider`, not
a getter). The remaining `instanceof` branches cover the actual
component hierarchy and tests pass.
- `Element.insertBefore`'s parent comparison uses `.equals()`
(Vaadin's Element overrides equals) rather than `==`, fixing a
bug the agent caught by running the test.
- `checkEditableByUser` calls `component.isAttached()` (the
overridable instance method) rather than a static helper, so test
subclasses' overrides are honored.
- `TestingLifecycleHook` becomes an interface with a default static
factory `DEFAULT`; the mutable global `testingLifecycleHook` lives
on `TestingLifecycleHooks.current` (separate utility class) since
Java interfaces can't hold mutable static fields.
Runtime impact: still no Kotlin dependency drop on this branch — the
remaining files (Locator, MockVaadin, PrettyPrintTree, Routes, plus
Matches.kt and component/Grid.kt) keep kotlin-stdlib + kotlin-reflect
on the classpath until Phases 3–5 land.
LOC delta: 8 .kt deleted (~1,179 LOC), 9 .java added (~1,729 LOC) +
1 .kt kept for the SearchSpec-deferred Matches helper (24 LOC).
Expansion ratio ≈ +47%, in line with Phase 1's +54%.
Verified by all four test suites after a clean rebuild:
- shared: 20 tests
- junit6: 1007 tests
- spring: 29 tests
- quarkus: 27 tests
All green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3 of the de-Kotlin effort: the two larger Kotlin utility files
in `shared/src/main/kotlin/com/vaadin/browserless/internal/` —
`PrettyPrintTree.kt` (214 LOC) and `Routes.kt` (199 LOC) — become Java
files in `shared/src/main/java/com/vaadin/browserless/internal/`.
Mechanical translation, no behavior change.
The two `open class` declarations Kotlin allowed inside `Routes.kt`
(MockRouteNotFoundError and MockInternalSeverError — the latter
spelled as in the existing codebase) are split into their own .java
files since Java only allows one public top-level class per file.
Package-internal API surface is unchanged.
PrettyPrintTree's old `kotlin.reflect.jvm.kotlinFunction` block (the
`href` accessor lookup on arbitrary components) is replaced with a
plain `java.lang.reflect.Method` lookup that matches both `href()`
and `getHref()` zero-arg signatures, with a null/blank guard so
unset Anchors still print as `Anchor[]` (Anchor.getHref() returns ""
where the original Kotlin-reflect path read the private field
directly and saw null). This is the same fix that was on the
intermediate `feat/no-kotlin-lib` branch — that branch is now
subsumed by this one.
With both Kotlin reflection users gone, the kotlin-reflect dependency
(~3.3 MB) drops from shared/pom.xml. `kotlin-stdlib` and friends stay
on the classpath until later phases — Grid.kt's KClass/KProperty1
references and Matches.kt's IntRange.size are type-level only and are
satisfied by kotlin-stdlib alone, verified via `mvn dependency:tree`.
Cross-call-site updates:
- Java: 11 sites + 3 import lines across ComponentTester,
internal/BasicUtils, internal/Renderers, MenuBarTester,
ContextMenuTester — all `PrettyPrintTreeKt.*` calls become
`PrettyPrintTree.*`.
- Kotlin production: Locator.kt (4 sites), component/Grid.kt
(5 sites + 1 import swap).
- Kotlin tests: MockVaadinTest.kt (2), RoutesTest.kt (1 cast fix
+ explicit set type annotations to substitute for what Kotlin's
data-class type inference used to provide), PrettyPrintTreeTest.kt
(2 file-local extension shims that delegate to the Java statics —
keeps 34 test call sites readable without 34 inline rewrites).
Notable design decisions:
- Routes Java port generates explicit `equals`, `hashCode`,
`toString`, `copy(Set, Set, Set, boolean)` to match Kotlin's data
class semantics. Multiple constructors (no-arg, 2-arg, 3-arg,
4-arg) match the bytecode-overload surface Kotlin produced.
- prettyPrintUseAscii / prettyStringHook / dontDumpAttributes
become public static mutable fields on PrettyPrintTree (matching
Kotlin's top-level var semantics).
- prettyPrintUseAscii is now read per-print instead of cached at
PrettyPrintTree construction. Strictly more correct — mutating
the flag between constructing a tree and printing it now actually
affects the output, where the Kotlin original captured it once at
construction. No test catches this; behavior change is arguably
more user-friendly.
Validation:
- shared: 20 tests
- junit6: 1007 tests
- spring: 29 tests
- quarkus: 27 tests
All green after `rm -rf shared/target` + clean install.
`mvn dependency:tree -pl shared` confirms kotlin-reflect is absent
and kotlin-stdlib (+jdk7/jdk8) remain.
LOC delta: 2 Kotlin files deleted (~425 LOC), 4 Java files added
(~616 LOC). Plus the pom.xml block removal.
After this phase, the Kotlin files left in `shared/src/main/kotlin/`
are: Locator.kt, MockVaadin.kt, Matches.kt (in internal/) and Grid.kt
(in component/). Phases 4 and 5 cover those.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 4 of the de-Kotlin effort: the locator engine (Locator.kt, 418
LOC) and the central setup/teardown utility (MockVaadin.kt, 581 LOC)
become Java in `shared/src/main/java/com/vaadin/browserless/internal/`.
`Matches.kt` from Phase 2 is replaced by a 138-LOC `LocatorDsl.kt`
that preserves Kotlin DSL syntax for test callers.
Files produced:
- Locator.java (376 LOC) — top-level statics: `_get`, `_find`,
`_expect*`, `_walkAll`,
`_expectInternalServerError`,
`_expectNoDialogs`, `currentPath`.
- SearchSpec.java (364 LOC) — split out of Locator: public class
with mutable fields + `toPredicate()`.
- MockVaadin.java (685 LOC) — public final class with private
constructor + static methods,
mirroring the Kotlin `object`.
Includes nested public static
`MockPage` class.
- SessionObjects.java (86 LOC) — data-class-equivalent POJO with
explicit equals/hashCode/toString/copy.
- UIFactory.java (28 LOC) — @FunctionalInterface, `UI invoke()`.
Method name kept as `invoke()` to
preserve binary compatibility with
the previous `Function0<UI>` SAM
shape — see the divergence note in
reviewer-notes-phase-4.
- MockRequestCustomizer.java — small @FunctionalInterface.
(22 LOC)
Kotlin shim (replaces Matches.kt):
- LocatorDsl.kt (138 LOC) — top-level Kotlin functions that bridge
`SearchSpec<T>.() -> Unit` DSL block syntax into the Java engine's
`Consumer<SearchSpec<T>>` API. Keeps tests like
`Button()._get(Button::class.java) { caption = "bar" }` working
without an inline rewrite of 20+ test call sites.
Also holds the deferred `Component.matches(spec)` extension and the
`IntRange.size` helper used by Grid.kt — both will move/disappear
in Phase 5.
Notable design decisions / divergences from the original brief:
- UIFactory's SAM is `invoke()` rather than `create()`. Preserves
binary compatibility with the previous `Function0<UI>` shape on
the bytecode, so callers using `MockedUI::new` keep working with
zero changes.
- Three `@Deprecated(forRemoval=true)` `Function0<UI>` constructors
were removed from Spring's MockSpringServlet, MockSpringServletService,
MockSpringVaadinSession. They became ambiguous with the new
UIFactory overloads at every `MockedUI::new` call site, and had
no internal callers. Reviewers should sign off on this small
breaking-API removal (the constructors were already flagged for
removal in the API).
- SearchSpec.toPredicate() returns `Predicate<T>` (Java). The Kotlin
original returned `(T) -> Boolean`. Test call sites that did
`spec.toPredicate()(component)` now do
`spec.toPredicate().test(component)`. ~6 lines updated in
SearchSpecTest.kt.
- `MockVaadin.runUIQueue` rethrows the captured `Throwable[0]` via
a `sneakyThrow` cast (the standard Java pattern). Necessary because
Java distinguishes checked vs unchecked exceptions where Kotlin
doesn't. Without sneakyThrow, AsyncTest.clientRoundtrip()
propagates failures would wrap the cause in a RuntimeException and
the `expectThrows(ExecutionException::class)` assertion would fail.
- `SearchSpec.count` keeps its `kotlin.ranges.IntRange` field type
(matching the original Kotlin source and existing Java caller in
ComponentQuery). Replacing it would widen the diff into
ComponentQuery and into every test that constructs a SearchSpec.
- Added a one-arg `MockVaadin.setup(UIFactory)` overload not strictly
in the brief — needed for unambiguous test resolution against
the existing `setup(Routes)` / `setup(VaadinServlet)` overloads.
Cross-call-site updates:
- Java: ComponentQuery (4 sites, LocatorKt → Locator, dropped
kotlin.Unit import), BrowserlessUIContext (1 import line for
nested MockPage class), 3 Spring servlet/service/session files
(deprecated ctor removals).
- Kotlin tests: MockVaadinTest.kt (4 sites — Kotlin can't use
named-arg syntax against Java methods without `-parameters`, so
`setup(uiFactory = ...)` becomes `setup(UIFactory { ... })`);
SearchSpecTest.kt (110-line diff: named-arg constructor calls
rewritten as `.apply { ... }`, predicate invocation switched to
`.test(c)`). LocatorTest.kt — zero changes, the LocatorDsl.kt
shim makes its DSL syntax keep working.
Validation:
- shared: 20/20
- junit6: 1007/1007
- spring: 29/29
- quarkus: 27/27
All green after a clean rebuild. `mvn dependency:tree -pl shared`
confirms kotlin-reflect still absent and kotlin-stdlib (+jdk7/jdk8)
still present (needed by Grid.kt and LocatorDsl.kt until Phase 5).
After this phase, the Kotlin files left in `shared/src/main/kotlin/`:
- internal/LocatorDsl.kt (138 LOC) — DSL conveniences
- component/Grid.kt (837 LOC) — Phase 5 territory
Total remaining ≈ 975 LOC. ~75% of the original Kotlin footprint is
now Java.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 5 (final) of the de-Kotlin effort: the heaviest single Kotlin file (component/Grid.kt, 837 LOC) is ported to Java. The Kotlin DSL shim that survived Phase 4 (internal/LocatorDsl.kt) moves to test scope. With no Kotlin remaining in shared's main sources, the kotlin-stdlib dependency drops out of the compile classpath entirely. After this commit, `mvn dependency:tree -pl shared -Dscope=compile` shows zero `org.jetbrains.kotlin:*` entries. Downstream consumers of the shared module no longer pull the Kotlin runtime transitively. The Kotlin compiler stays in shared's build for test compilation (20 tests in src/test/kotlin/ remain Kotlin per the de-kotlin-plan open question #1). Files: - `shared/src/main/java/com/vaadin/browserless/component/Grid.java` (1120 LOC) — port of Grid.kt. ~45 top-level Kotlin extensions become Java statics. Heavy reflection on DataCommunicator, ColumnPathRenderer, AbstractCell, TreeGrid internals — all private static Method/Field caches with try/catch around ReflectiveOperationException. Sequence<T> (`_rowSequence`) ported to Stream<T> via DepthFirstTreeIterator + Spliterator to keep laziness for callers that consume with .count() / .skip().limit(). - `shared/src/main/java/com/vaadin/browserless/internal/CountRange.java` (95 LOC) — new pure-Java helper. Replaces uses of `kotlin.ranges.IntRange` in SearchSpec.count and ComponentQuery.LocatorSpec.count. Necessary because Phase 4 left IntRange in those public fields and moving kotlin-stdlib to test scope would have broken compilation otherwise. - `junit6/src/test/kotlin/com/vaadin/browserless/internal/LocatorDsl.kt` (123 LOC) — moved from shared/src/main/kotlin/ to junit6 test scope. Houses the Kotlin inline-reified + DSL-block conveniences (`_get`, `_find`, `_expectOne`, etc.) and `Component.matches`. 98 call sites in LocatorTest.kt resolve unchanged because the new location is in the same package. - `shared/pom.xml` — removes main-Kotlin compile execution, removes `add-kotlin-sources-for-source-jar` build-helper execution, scopes `kotlin-stdlib-jdk8` to test, removes the dokka plugin entirely (replaced by maven-javadoc-plugin in the release profile). - `shared/src/main/kotlin/` directory deleted (was empty). Design decisions / divergences from the brief: - **CountRange replaces IntRange.** This is a public-API breaking rename in `SearchSpec.count` and `ComponentQuery.LocatorSpec.count` field types. No existing test reads/writes those fields directly, so the change is invisible to current callers, but downstream consumers constructing a SearchSpec by setting count would need to update. Worth a reviewer sign-off — this is the second small breaking change in the four-phase chain (the first was the Phase 4 removal of three @deprecated Spring constructors). - **`org.jetbrains.annotations.@NotNull/@Nullable` dropped from 4 Java files** (AccordionTester, CheckboxGroupTester, RadioButtonGroupTester, plus one source). They came transitively via kotlin-stdlib. Documentation-only annotations; no behavior impact. Geolocation files still use JSpecify's `@Nullable` which is a separate dependency unaffected. - **`Grid` Java class name kept** despite simple-name clash with `com.vaadin.flow.component.grid.Grid`. GridTester.java uses the FQN `com.vaadin.browserless.component.Grid.method(...)` for the 6 call sites instead of an import. Renaming to `Grids` was considered but `Grid` matches the original Kotlin file name and the precedent of Routes/Locator (singular nouns matching the Kotlin source). - **`KProperty1<*, *>` overloads dropped** from `HeaderRow.getCell` and `FooterRow.getCell`. Zero callers across the codebase. - **Dokka plugin removed entirely.** With no main Kotlin, dokka is dead weight. Release profile now uses `maven-javadoc-plugin` instead. The `dokka-javadocs` profile (only active with `-Djavadocs`) is removed too. Validation: - shared: 20/20 pass - junit6: 1007/1007 pass - spring: 29/29 pass - quarkus: 27/27 pass - `mvn dependency:tree -pl shared -Dscope=compile`: zero kotlin libs - `mvn dependency:tree -pl shared -Dscope=test`: kotlin-stdlib-jdk8 still present (needed for src/test/kotlin/) LOC delta: 2 .kt files removed from main (~975 LOC), 1 .kt moved to test scope (123 LOC after IntRange.size + unused filterNotBlank strip), 2 .java files added (~1215 LOC), pom slimmed by ~30 lines. This completes the de-Kotlin effort on `shared/src/main/`. The five phases together: 25 Kotlin files removed, ~5,100 LOC of Kotlin translated to ~7,800 LOC of Java + 123 LOC of test-scope Kotlin shim, one entire runtime dependency dropped (kotlin-reflect ~3.3 MB), and kotlin-stdlib drops from downstream consumers' transitive classpath. Co-Authored-By: Claude Opus 4.7 (1M context) <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
1 participant
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.
Unplanned experiment that started from getting rid of kotlin-reflect (small change), ended up vibing out also the rest of kotlin library dependencies.
No value for those who use this for testing only, but some help for those who (ab)use this for something else than testing 🤓