Skip to content

feat(flow-client): port the engine assembly to TypeScript - #24952

Draft
totally-not-ai[bot] wants to merge 8 commits into
flow-client-ts-networkfrom
flow-client-ts-engine
Draft

feat(flow-client): port the engine assembly to TypeScript#24952
totally-not-ai[bot] wants to merge 8 commits into
flow-client-ts-networkfrom
flow-client-ts-engine

Conversation

@totally-not-ai

@totally-not-ai totally-not-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Ports the top layer that wires the client together: DefaultRegistry (the DI hub instantiating every service), ApplicationConnection (the engine entry via ApplicationConnection.create), Bootstrapper, and the published client API (publishClient, clientApi). Completes the TypeScript port — all 80 internal engine modules are now present and unit-tested.

Additive only — nothing is wired into the running client (GWT remains the live engine); exercised by mocha (23 tests). tsc + eslint clean.

Stacked on #24951 (base flow-client-ts-network) — merge after it. This PR's diff is only its own new files (additive, no edits to existing code), so it can be reviewed independently of the merge order.

⚠️ Non-draft, but please review before merging.

@totally-not-ai

totally-not-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Review — 1:1 Java→TS port fidelity

Reviewed the engine-assembly files against ApplicationConnection.java, the bootstrap logic, DefaultRegistry.java, the publishJavascriptMethods/publishDevelopmentModeJavascriptMethods JSNI, and GwtApplicationConnectionTest.

Production code — faithful ✅

  • DefaultRegistry.ts — cross-checked the full registration set against the Java constructor: all 22 services are present and in the exact same order, with matching resettable-vs-final classification (UILifecycle/ConstantPool/ExistingElementMap/Heartbeat resettable, the rest final). The import-direction adaptations are correctly wired (lazy SystemErrorHandler adapter for ResourceLoader, getServerEventObjectForResync for StateTree, atmospherePushConnectionFactory for MessageSender). No missing or reordered registrations.
  • ApplicationConnection.ts, publishClient.ts, clientApi.ts — faithful. Instance API, getProfilingData (now on MessageHandler, same -1,-1 fallback), the published client members, the request-timing gate, and the dev-mode block (getVersionInfo/debug/getNodeInfo) all match the JSNI. JAVA_CLASS='class' and the app-id -\d+$ strip are correct.
  • Bootstrapper.ts — the start/defer/callback sequence and serviceUrl/contextRootUrl resolution match.

Intentional/safe divergences (worth confirming, none blocking):

  • The construction order is inverted — TS builds the registry first and the ApplicationConnection last, so the Java set(ApplicationConnection.class, connection) self-registration is dropped. Verified safe: getApplicationConnection() has zero consumers anywhere. This is the unavoidable resolution of the import-direction constraint.
  • getRunningApplications() / the static runningApplications list is dropped — verified to have no consumers (dead API).

Minor findings (please confirm intent)

  • MINOR — onModuleLoad warning divergence. Java initModule warns in both the already-loaded and bootstrap-missing cases (if (moduleLoaded || !vaadinBootstrapLoaded())). The TS only warns when the bootstrap is missing and returns silently when already bootstrapped. Restore the moduleLoaded || branch or confirm the silent return is deliberate.
  • MINOR — null→default coercions in populateApplicationConfiguration. Java stores raw null for absent webcomponents (→ exportedWebComponents = null) and for liveReloadUrl/liveReloadBackend/springBootLiveReloadPort; the TS coerces to []/''. So published client.exportedWebComponents is null in Java vs [] in TS. Confirm no consumer distinguishes null from empty.
  • NITgetConfigInteger returns 0 for missing where Java returns nullable Integer (bootstrap always supplies these); dropped Profiler.* calls (documented as intentional).

Tests

BootstrapperTests, DefaultRegistryTests, PublishClientTests have no Java baseline and provide solid net-new coverage (suffix-strip, production-mode omission of dev/profiling methods, reset semantics, cross-wiring). ApplicationConnectionTests.ts is far richer than the Java side, but:

  • MAJOR — missing the one Java @test. GwtApplicationConnectionTest.test_should_not_addNavigationEvents_forWebComponents (asserts that a web-component-mode bootstrap wires no popstate listener on window and no click on body) has no TS equivalent — a grep for popstate/addNavigationEvents across src/test/frontend is empty. Please port it, or record why it is out of scope (e.g. the router navigation wiring isn't part of this PR yet).

Recommendation: production wiring is good to go pending confirmation of the two MINOR null/warning deviations. On tests, port (or explicitly defer with a note) the test_should_not_addNavigationEvents_forWebComponents case.

Artur- added 2 commits July 8, 2026 20:00
Ports the top layer that wires the client together: DefaultRegistry (the DI hub
that instantiates every service), ApplicationConnection (the engine entry via
ApplicationConnection.create), Bootstrapper, and the published client API
(publishClient, clientApi).

Completes the TypeScript port of the client engine -- all 80 internal modules are
now present and unit-tested. Still additive: nothing is wired into the running
client (GWT remains live); the cutover is a separate step. Depends on PR1-6.
Exercised by mocha (23 tests).
@totally-not-ai
totally-not-ai Bot force-pushed the flow-client-ts-engine branch from fbfbe84 to 760dd0a Compare July 8, 2026 17:01
@totally-not-ai
totally-not-ai Bot force-pushed the flow-client-ts-network branch from a5a34a8 to 6244dc5 Compare July 8, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants