diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c73f2af4..69a3015c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,13 @@ on: - main workflow_dispatch: +# Superseded PR runs are wasted compute: when a new push lands on a PR, cancel +# the in-flight run for the previous commit. Pushes to main are never +# cancelled — every main commit keeps its complete CI record for bisecting. +concurrency: + group: ci-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: ci-linux: name: CI / Linux (Android, Desktop, wasmJs) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4f381c3c..7defbd19 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,26 +6,26 @@ on: description: 'Version Name' required: true default: '' - changes: - description: 'Release notes' - required: true - default: '' jobs: release: name: Release runs-on: macos-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 + with: + # add-and-commit pushes the release commit using the checkout + # credentials, so check out with the publish token. + token: ${{ secrets.PUBLISH_GITHUB_TOKEN }} - name: Set up JDK 21 - uses: actions/setup-java@v3.9.0 + uses: actions/setup-java@v4 with: distribution: 'zulu' java-version: 21 - name: Setup gradle - uses: gradle/gradle-build-action@v2 + uses: gradle/actions/setup-gradle@v4 - name: Install gpg secret key run: cat <(echo -e "${{ secrets.PUBLISH_SIGNING_KEY_LITERAL }}") | gpg --batch --import @@ -36,8 +36,13 @@ jobs: - name: Export gpg secret key run: sudo gpg --export-secret-keys --pinentry-mode loopback --passphrase=${{ secrets.PUBLISH_SIGNING_KEY_PASSWORD }} ${{ secrets.PUBLISH_SIGNING_KEY_ID }} > ${{ secrets.PUBLISH_SIGNING_KEY_LOCATION }} - - name: Update Version Name - run: ./gradlew updateVersion -PversionName=${{ github.event.inputs.versionname }} + # Stamps version.properties AND the changelog: the standing + # "## Unreleased" section becomes "## ()", a fresh empty + # "## Unreleased" is inserted above it, and the released section's body + # is written to build/release-notes.md for the Create Release step. + # Fails (before writing anything) if the Unreleased section is empty. + - name: Update Version Name + Changelog + run: ./gradlew updateVersion -PversionName=${{ inputs.versionname }} - name: Publish Release env: @@ -51,21 +56,15 @@ jobs: run: ./gradlew publishAllPublicationsToMavenCentralRepository -PmavenCentralUsername="${{ secrets.PUBLISH_SONATYPE_USER }}" -PmavenCentralPassword="${{ secrets.PUBLISH_SONATYPE_PASSWORD }}" --no-parallel --stacktrace --continue --exclude-task :enro-common:publishJsPublicationToMavenCentralRepository - name: Update Repo - uses: EndBug/add-and-commit@v5 - env: - GITHUB_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} + uses: EndBug/add-and-commit@v9 with: - add: "./version.properties" - message: ${{ format('Released {0}', github.event.inputs.versionname) }} + add: | + version.properties + CHANGELOG.md + message: ${{ format('Released {0}', inputs.versionname) }} push: true - name: Create Release - uses: actions/create-release@v1 env: - GITHUB_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} - with: - tag_name: ${{ github.event.inputs.versionname }} - release_name: Release ${{ github.event.inputs.versionname }} - body: ${{ github.event.inputs.changes }} - draft: false - prerelease: false + GH_TOKEN: ${{ secrets.PUBLISH_GITHUB_TOKEN }} + run: gh release create "${{ inputs.versionname }}" --title "Release ${{ inputs.versionname }}" --notes-file build/release-notes.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 76a3738c..55e11cab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,43 @@ # Changelog -## 3.0.0-beta02 (Unreleased) +## Unreleased + +### Web browser history (WasmJS) + +* Rewrote `WebHistoryPlugin`'s history synchronisation. Updates are now + processed on a serial queue instead of being dropped while a sync was in + flight (the cause of a single browser back traversing multiple screens), + self-initiated `history.go()` traversals await and consume their popstate + echo instead of racing a `delay(1)` (the cause of double-pops), and + full-backstack replacements (e.g. a loading gate resolving to home, or a + section switch) now `replaceState` instead of `pushState`, so transient + screens can no longer survive as browser back targets. +* History entries whose recorded state can't be decoded (e.g. written by an + older build of an app — tab history survives deploys) now restore through + the entry's URL via the `@NavigationPath` bindings instead of doing + nothing, and self-heal the entry's stored state. +* Serialized history state is verified to round-trip at write time; a state + that can't restore is a hard error rather than a silently broken back + button later. + +### Serialization + +* `EnroController.jsonConfiguration` now uses kotlinx's default + (`POLYMORPHIC`) class-discriminator mode instead of `ALL_JSON_OBJECTS`. + With kotlinx 1.11, `ALL_JSON_OBJECTS` breaks both Json encoders for + realistic `NavigationKey` shapes under polymorphic dispatch: the streaming + encoder leaks a value-class field's deferred discriminator into the + next-opened object (corrupting `Instance.metadata` so persisted state + could not be decoded) and emits invalid JSON for collection fields, while + the tree encoder crashes on collection fields. `POLYMORPHIC` mode writes + discriminators exactly where polymorphic deserialization reads them and + handles all of these shapes correctly. `HistoryStateSerializationTests` + documents the upstream kotlinx failures + ([Kotlin/kotlinx.serialization#3022](https://github.com/Kotlin/kotlinx.serialization/issues/3022)) + so a fixed kotlinx release is detected. SavedState serialization (`ALL_OBJECTS`, androidx encoder) is + unaffected and covered by `SavedStateSerializationTests`. + +## 3.0.0-beta02 ### Predictive back diff --git a/build.gradle.kts b/build.gradle.kts index ddc5051c..aadc913f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -115,6 +115,44 @@ tasks.register("updateVersion") { error("The versionName '$versionName' is the current versionName") } + // ------------------------------------------------------------------ + // Changelog: changes stack under a standing "## Unreleased" header. + // Releasing stamps that header with the version (and date), inserts a + // fresh empty "## Unreleased" above it, and writes the released + // section's body to build/release-notes.md for the release workflow + // to attach to the GitHub release. All validation happens before any + // file is written, so a failed release leaves the repo untouched. + // ------------------------------------------------------------------ + val changelogFile = rootProject.file("CHANGELOG.md") + val changelog = changelogFile.readText() + val unreleasedHeader = "## Unreleased" + val headerIndex = changelog.indexOf(unreleasedHeader) + if (headerIndex == -1) { + error("CHANGELOG.md must contain an '## Unreleased' section to release from") + } + val bodyStart = headerIndex + unreleasedHeader.length + val nextHeaderIndex = changelog.indexOf("\n## ", bodyStart) + .let { if (it == -1) changelog.length else it } + val releaseNotesBody = changelog.substring(bodyStart, nextHeaderIndex).trim() + if (releaseNotesBody.isEmpty()) { + error( + "The 'Unreleased' section of CHANGELOG.md is empty — " + + "add release notes before releasing $versionName" + ) + } + + val releaseNotesFile = rootProject.layout.buildDirectory.file("release-notes.md").get().asFile + releaseNotesFile.parentFile.mkdirs() + releaseNotesFile.writeText(releaseNotesBody + "\n") + + val releaseDate = java.time.LocalDate.now() + changelogFile.writeText( + changelog.replaceFirst( + unreleasedHeader, + "## Unreleased\n\n## $versionName ($releaseDate)", + ) + ) + versionPropertiesFile.writeText("versionName=$versionName\nversionCode=$versionCode") } } diff --git a/enro-runtime/build.gradle.kts b/enro-runtime/build.gradle.kts index 19f7d557..c79968d5 100644 --- a/enro-runtime/build.gradle.kts +++ b/enro-runtime/build.gradle.kts @@ -14,8 +14,11 @@ kotlin { // Return default values for unmocked Android framework methods // rather than throwing "Method ... not mocked" — defensive against // any test path that brushes a stub from android.jar (e.g. - // savedstate's Bundle). + // savedstate's Bundle). Tests that genuinely exercise framework + // classes should extend dev.enro.test.platform.RobolectricHostTest + // to run under Robolectric on this target instead of the stubs. isReturnDefaultValues = true + isIncludeAndroidResources = true } } } @@ -57,6 +60,11 @@ kotlin { implementation(project(":enro-test")) implementation(libs.compose.uiTest) } + val androidHostTest by getting { + dependencies { + implementation(libs.testing.robolectric) + } + } androidMain.dependencies { implementation(libs.androidx.core) implementation(libs.androidx.appcompat) diff --git a/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..760d98c0 --- /dev/null +++ b/enro-runtime/src/androidHostTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,7 @@ +package dev.enro.test.platform + +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt index cf9f5bc9..6eb738fa 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/EnroController.kt @@ -98,6 +98,15 @@ public class EnroController { return instance ?: error("EnroController has not been installed") } + /** + * The controller's Json configuration, including all registered + * serializers. Uses the default (POLYMORPHIC) class-discriminator + * mode — `ALL_JSON_OBJECTS` is incompatible with kotlinx 1.11 for + * keys containing value-class or collection fields + * (https://github.com/Kotlin/kotlinx.serialization/issues/3022); see + * the note on SerializerRepository.jsonConfiguration and + * HistoryStateSerializationTests in enro-runtime. + */ public val jsonConfiguration: Json get() { val instance = requireInstance() return instance.serializers.jsonConfiguration diff --git a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt index 33d892ed..a3d12a12 100644 --- a/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt +++ b/enro-runtime/src/commonMain/kotlin/dev/enro/controller/repository/SerializerRepository.kt @@ -45,7 +45,20 @@ internal class SerializerRepository { var jsonConfiguration: Json = Json { serializersModule = this@SerializerRepository.serializersModule - classDiscriminatorMode = kotlinx.serialization.json.ClassDiscriminatorMode.ALL_JSON_OBJECTS + // Deliberately the default (POLYMORPHIC) discriminator mode. + // ALL_JSON_OBJECTS is unusable with kotlinx 1.11 for realistic + // NavigationKey shapes: under polymorphic dispatch, the STREAMING + // encoder leaks a value-class field's deferred discriminator into + // the next-opened object (corrupting Instance.metadata), and emits + // INVALID JSON for collection fields (a "type" key:value pair + // inside an array); the TREE encoder crashes outright + // (NumberFormatException) on collection fields. POLYMORPHIC mode + // writes discriminators exactly where polymorphic deserialization + // reads them (Instance.key, metadata values) and handles all of + // these shapes correctly. See HistoryStateSerializationTests, + // which also documents the ALL_JSON_OBJECTS failures so a kotlinx + // upgrade that fixes them is detected. Upstream: + // https://github.com/Kotlin/kotlinx.serialization/issues/3022 ignoreUnknownKeys = true } private set diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt new file mode 100644 index 00000000..020023fe --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/HistoryStateSerializationTests.kt @@ -0,0 +1,141 @@ +package dev.enro.serialization + +import dev.enro.NavigationKey +import dev.enro.asInstance +import dev.enro.controller.repository.SerializerRepository +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.PolymorphicSerializer +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.ClassDiscriminatorMode +import kotlinx.serialization.json.Json +import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass +import kotlin.jvm.JvmInline +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@Serializable +@JvmInline +internal value class HistoryTestId(val value: String) + +@Serializable +internal enum class HistoryTestType { NPC, LOCATION } + +@Serializable +internal data class HistoryTestKey( + val id: HistoryTestId, + val name: String = "", + val types: Set = emptySet(), + val excludeIds: Set = emptySet(), +) : NavigationKey + +/** + * Pins the serialization of persisted navigation state (browser history, + * deep-link tooling) under the controller's json configuration, which uses + * the default (POLYMORPHIC) class-discriminator mode. + * + * `ClassDiscriminatorMode.ALL_JSON_OBJECTS` cannot be used: with kotlinx + * 1.11, realistic NavigationKey shapes break BOTH encoders under polymorphic + * dispatch — + * + * * STREAMING: a value-class field's discriminator write is deferred until + * the next `beginStructure`, which an inline field never opens, so the + * pending discriminator leaks into the next-opened object + * (`Instance.metadata` in practice) and the output fails to decode with + * "Expected JsonObject, but had JsonLiteral". Collection fields produce + * outright INVALID JSON (a `"type"` key:value pair inside an array). + * * TREE (`encodeToJsonElement`): collection fields crash the encoder with + * `NumberFormatException: For input string: "type"` (the deferred + * discriminator is applied inside a list context, where the tag is parsed + * as an array index). + * + * The documenting tests below assert those failures still exist: when one + * starts failing, kotlinx has fixed the corresponding bug and the + * POLYMORPHIC-mode constraint can be revisited. Upstream issue: + * https://github.com/Kotlin/kotlinx.serialization/issues/3022 + */ +class HistoryStateSerializationTests { + + private fun repository(): SerializerRepository { + return SerializerRepository().apply { + registerSerializersModule( + SerializersModule { + polymorphic(NavigationKey::class) { + subclass(HistoryTestKey.serializer()) + } + } + ) + } + } + + private val instanceSerializer = + NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) + + private val fullShapeKey = HistoryTestKey( + id = HistoryTestId("abc-123"), + name = "name", + types = setOf(HistoryTestType.NPC, HistoryTestType.LOCATION), + excludeIds = setOf(HistoryTestId("excluded")), + ) + + @Test + fun instanceWithValueClassAndCollectionFieldsRoundTrips() { + val json = repository().jsonConfiguration + val instance = fullShapeKey.asInstance() + + val encoded = json.encodeToString(instanceSerializer, instance) + val decoded = json.decodeFromString(instanceSerializer, encoded) + + assertEquals(instance.id, decoded.id) + assertEquals(instance.key, decoded.key) + } + + @OptIn(ExperimentalSerializationApi::class) + private fun allJsonObjectsJson(): Json = Json(from = repository().jsonConfiguration) { + classDiscriminatorMode = ClassDiscriminatorMode.ALL_JSON_OBJECTS + } + + /** + * Documents the upstream kotlinx streaming-encoder bug. If this test + * FAILS, kotlinx has fixed the deferred-discriminator leak for inline + * fields and ALL_JSON_OBJECTS may be viable again (check the other + * documenting tests too). + */ + @Test + fun allJsonObjectsStreamingEncoderLeaksValueClassDiscriminator() { + val json = allJsonObjectsJson() + val instance = HistoryTestKey(id = HistoryTestId("abc-123")).asInstance() + + val encoded = json.encodeToString(instanceSerializer, instance) + + assertTrue( + encoded.contains("\"metadata\":{\"type\":\"dev.enro.serialization.HistoryTestId\""), + "kotlinx appears to have fixed the streaming-encoder discriminator leak — " + + "ALL_JSON_OBJECTS may be viable again. Encoded: $encoded", + ) + } + + /** + * Documents the upstream kotlinx tree-encoder bug. If this test FAILS, + * kotlinx has fixed the collection-field discriminator crash and + * ALL_JSON_OBJECTS may be viable again (check the other documenting + * tests too). + */ + @Test + fun allJsonObjectsTreeEncoderFailsOnCollectionFields() { + val json = allJsonObjectsJson() + val instance = fullShapeKey.asInstance() + + val result = runCatching { + json.encodeToJsonElement(instanceSerializer, instance) + } + + assertTrue( + result.isFailure, + "kotlinx appears to have fixed the tree-encoder collection-field crash — " + + "ALL_JSON_OBJECTS may be viable again. Encoded: ${result.getOrNull()}", + ) + } +} diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt new file mode 100644 index 00000000..36494319 --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/serialization/SavedStateSerializationTests.kt @@ -0,0 +1,53 @@ +package dev.enro.serialization + +import androidx.savedstate.serialization.decodeFromSavedState +import androidx.savedstate.serialization.encodeToSavedState +import dev.enro.NavigationKey +import dev.enro.asInstance +import dev.enro.controller.repository.SerializerRepository +import dev.enro.test.platform.RobolectricHostTest +import kotlinx.serialization.PolymorphicSerializer +import kotlinx.serialization.modules.SerializersModule +import kotlinx.serialization.modules.polymorphic +import kotlinx.serialization.modules.subclass +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The savedstate analogue of [HistoryStateSerializationTests]: container + * backstacks (rememberNavigationContainer), flow results, and enroSaver all + * round-trip through androidx.savedstate serialization with + * `ClassDiscriminatorMode.ALL_OBJECTS`. This pins that an Instance whose key + * carries a value-class field survives that round-trip — i.e. that the + * androidx SavedState encoder does NOT share the kotlinx streaming-JSON + * encoder's deferred-discriminator leak (see HistoryStateSerializationTests). + */ +class SavedStateSerializationTests : RobolectricHostTest() { + + private fun repository(): SerializerRepository { + return SerializerRepository().apply { + registerSerializersModule( + SerializersModule { + polymorphic(NavigationKey::class) { + subclass(HistoryTestKey.serializer()) + } + } + ) + } + } + + private val instanceSerializer = + NavigationKey.Instance.serializer(PolymorphicSerializer(NavigationKey::class)) + + @Test + fun instanceWithValueClassKeyFieldRoundTripsThroughSavedState() { + val configuration = repository().savedStateConfiguration + val instance = HistoryTestKey(id = HistoryTestId("abc-123"), name = "name").asInstance() + + val saved = encodeToSavedState(instanceSerializer, instance, configuration) + val decoded = decodeFromSavedState(instanceSerializer, saved, configuration) + + assertEquals(instance.id, decoded.id) + assertEquals(instance.key, decoded.key) + } +} diff --git a/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..fd90d35c --- /dev/null +++ b/enro-runtime/src/commonTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,15 @@ +package dev.enro.test.platform + +/** + * Base class for common tests that touch Android framework classes (Bundle, + * SavedState) at runtime. + * + * On the Android host-test target, framework classes come from android.jar + * stubs, and `isReturnDefaultValues = true` makes them silently return + * defaults — e.g. `Bundle.getBundle(...)` returns null for a value that was + * "written" moments earlier, which surfaces as confusing downstream errors + * ("No valid saved state was found for the key ..."). Extending this class + * runs the test under Robolectric on that target, providing functional + * framework implementations; on every other target it is a no-op. + */ +expect abstract class RobolectricHostTest() diff --git a/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/desktopTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/iosTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor() diff --git a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt index c8bb8359..ecbb24d8 100644 --- a/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt +++ b/enro-runtime/src/wasmJsMain/kotlin/dev/enro/ui/WebHistoryPlugin.kt @@ -9,14 +9,18 @@ import dev.enro.annotations.ExperimentalEnroApi import dev.enro.context.ContainerContext import dev.enro.controller.createNavigationModule import dev.enro.emptyBackstack +import dev.enro.path.getBackstackFromPath import dev.enro.path.getPathFromNavigationKey import dev.enro.platform.EnroLog import dev.enro.plugin.NavigationPlugin import kotlinx.browser.window +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job -import kotlinx.coroutines.delay +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeout import kotlinx.coroutines.yield @@ -32,17 +36,47 @@ import org.w3c.dom.events.Event // // Nested URL routing is a known future direction; see docs/ghpages/docs/platform/web.md // for the model we ship in beta. +// +// Synchronisation model: every input (destination lifecycle callback or browser +// popstate) is enqueued onto a single serial processor, so updates are never dropped +// and the in-memory mirror of browser history can't silently diverge from the real +// session history. History traversals the plugin itself initiates (`history.go`) +// are awaited via their popstate echo, which is consumed before it can be mistaken +// for a user-initiated back/forward. @ExperimentalEnroApi internal class WebHistoryPlugin( private val window: Window, private val rootContainer: ContainerContext, ) : NavigationPlugin() { - private var activeHistoryJob: Job? = null - private var eventListenerEnabled = true - private val eventListener: (Event) -> Unit = { - if (eventListenerEnabled && it is PopStateEvent) { - updateHistoryState(it) + private val scope = CoroutineScope(Dispatchers.Main) + + /** + * Serial work queue. `null` means "the backstack changed, re-sync browser + * history"; a [PopStateEvent] means "the browser navigated, re-sync the + * backstack". Processing strictly in order is what keeps [historyStates] + * truthful — the previous implementation dropped events that arrived while + * a sync was in flight, which desynced the mirror and made a single + * browser back traverse multiple app screens. + */ + private val events = Channel(capacity = Channel.UNLIMITED) + + /** + * Set while the plugin is awaiting the popstate echo of its own + * `history.go()` call — see [traverse]. The next popstate completes it and + * is consumed instead of being enqueued as user navigation. + */ + private var pendingTraversal: CompletableDeferred? = null + + private val eventListener: (Event) -> Unit = { event -> + if (event is PopStateEvent) { + val traversal = pendingTraversal + if (traversal != null) { + pendingTraversal = null + traversal.complete(Unit) + } else { + events.trySend(event) + } } } @@ -50,24 +84,48 @@ internal class WebHistoryPlugin( private val historyStates = mutableListOf() private var historyIndex = -1 // Index of the current state in historyStates + private val processor: Job + init { window.addEventListener("popstate", eventListener) + processor = scope.launch { + for (event in events) { + try { + when (event) { + null -> syncFromBackstack() + else -> syncFromPopState(event) + } + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + // One failed sync must not kill history handling for the + // rest of the session — without this, a single throwing + // serializer/interceptor/path computation would end the + // processor loop and browser back would go silent while + // the URL keeps changing natively. + EnroLog.error("WebHistoryPlugin: history sync failed", t) + } + } + } } override fun onAttached(controller: EnroController) {} - override fun onDetached(controller: EnroController) {} + override fun onDetached(controller: EnroController) { + window.removeEventListener("popstate", eventListener) + processor.cancel() + } override fun onOpened(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } override fun onActive(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } override fun onClosed(navigationHandle: NavigationHandle<*>) { - updateHistoryState() + events.trySend(null) } /** @@ -91,136 +149,230 @@ internal class WebHistoryPlugin( return window.location.pathname + window.location.search } + /** + * Calls `history.go(delta)` and suspends until the browser delivers the + * resulting popstate, consuming that echo. `history.go` is asynchronous — + * the previous implementation `delay(1)`-ed and hoped, which raced the + * traversal (corrupting the history position) and let the echo arrive + * after suppression was lifted, where it was processed as a second + * user back. The timeout is a safety valve for browsers that elide the + * event (e.g. a no-op traversal at a history boundary). + */ + private suspend fun traverse(delta: Int) { + if (delta == 0) return + val deferred = CompletableDeferred() + pendingTraversal = deferred + window.history.go(delta) + try { + withTimeout(TRAVERSAL_TIMEOUT_MS) { deferred.await() } + } catch (t: TimeoutCancellationException) { + EnroLog.warn("WebHistoryPlugin: history traversal ($delta) produced no popstate within ${TRAVERSAL_TIMEOUT_MS}ms") + pendingTraversal = null + } + } + @OptIn(ExperimentalWasmJsInterop::class) - private fun updateHistoryState( - event: PopStateEvent? = null, - ) { - val container = rootContainer - eventListenerEnabled = false - if (activeHistoryJob != null) { + private fun decodeState(state: JsAny): ContainerNode? { + return runCatching { + EnroController.jsonConfiguration.decodeFromString(state.toString()) + }.onFailure { t -> + EnroLog.warn("WebHistoryPlugin: failed to decode history state (ignoring entry): ${t.message}") + }.getOrNull() + } + + /** + * The browser navigated (user back/forward): drive the backstack to match + * the entry's recorded state. When a recorded state can't be applied (an + * interceptor or EmptyBehavior refuses the close, or the app rewrote the + * backstack concurrently), step past it — bounded, rather than blind-firing + * `history.back()` and re-entering through the listener. + */ + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun syncFromPopState(event: PopStateEvent) { + // popstate without a state payload (manual address-bar edit, cross-origin + // nav). Under root-only routing we can't safely restore a sensible app + // state from URL alone — no-op and let the user reload if they want the + // URL to take effect. + val rawState = event.state ?: return + var poppedState = decodeState(rawState) + ?: return restoreFromUrl() + + var attempts = 0 + while (attempts < MAX_TRAVERSAL_ATTEMPTS) { + attempts++ + val currentState = createNodeFor(rootContainer) + if (currentState == poppedState) break + applyNodeFor(rootContainer, poppedState) + if (createNodeFor(rootContainer) == poppedState) break + // The recorded state didn't take — step one entry further back and + // try that one instead. + traverse(-1) + val nextRaw = window.history.state ?: return + poppedState = decodeState(nextRaw) + ?: return restoreFromUrl() + } + + val poppedIndex = historyStates.indexOfFirst { it == poppedState } + if (poppedIndex != -1) { + historyIndex = poppedIndex + } else { + historyStates.add(poppedState) + historyIndex = historyStates.lastIndex + } + } + + /** + * Fallback for a history entry whose recorded state can't be decoded — + * typically an entry written by an older build of the app whose + * serialization no longer matches (stale tab history survives deploys), + * or a metadata value that doesn't round-trip. Resolves the entry's URL + * through the controller's path bindings instead — degraded (single + * entry, same semantics as a cold-load deep link) but functional — and + * self-heals the entry by overwriting its unreadable state with the + * freshly serialized equivalent so the next visit decodes normally. + */ + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun restoreFromUrl() { + val fallback = rootContainer.controller.getBackstackFromPath(currentUrl()) + if (fallback == null) { + EnroLog.warn( + "WebHistoryPlugin: history entry state was unreadable and its URL " + + "('${currentUrl()}') has no path binding — leaving app state unchanged" + ) return } - activeHistoryJob = CoroutineScope(Dispatchers.Main).launch { - val currentState = createNodeFor(container) - val serializedCurrentState = EnroController.jsonConfiguration - .encodeToString(currentState) - .toJsString() - - val windowState = window.history.state?.let { - EnroController.jsonConfiguration - .decodeFromString(it.toString()) + applyNodeFor(rootContainer, ContainerNode( + containerKey = rootContainer.container.key, + backstack = fallback, + children = emptyList(), + )) + val currentState = createNodeFor(rootContainer) + val serializedCurrentState = serializeForHistory(currentState).toJsString() + window.history.replaceState(serializedCurrentState, "", computeUrl()) + val index = historyStates.indexOfFirst { it == currentState } + if (index != -1) { + historyIndex = index + } else { + historyStates.add(currentState) + historyIndex = historyStates.lastIndex + } + } + + /** + * The backstack changed (open/active/close): mirror it into browser history. + */ + /** + * Serializes [state] for storage in `history.state`, verifying the result + * actually decodes. Encode-and-decode-back is cheap insurance against + * serialization shapes kotlinx mishandles (see the discriminator-mode + * note on SerializerRepository.jsonConfiguration for the class of bug + * this guards against). + * + * Verification failure is a hard error — writing a state that can't + * restore would silently break browser back for the entry, and degrading + * (e.g. stripping metadata) would silently lose data such as + * result-channel wiring, which is worse than failing loudly. The error + * includes the live in-memory metadata: the serialized form mangles the + * offending entry, but the in-memory map still has the real keys and + * value types. + */ + @Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") + private fun serializeForHistory(state: ContainerNode): String { + val serialized = EnroController.jsonConfiguration.encodeToString(state) + val verification = runCatching { + EnroController.jsonConfiguration.decodeFromString(serialized) + } + if (verification.isSuccess) return serialized + + val metadataDescription = state.backstack.joinToString { instance -> + val entries = instance.metadata.map.entries.joinToString { (key, value) -> + "$key=${value::class.simpleName}" } + "${instance.key::class.simpleName}[$entries]" + } + error( + "WebHistoryPlugin: serialized history state failed round-trip verification " + + "(${verification.exceptionOrNull()?.message}). This entry would not restore on " + + "browser back, so it has NOT been written to history. In-memory metadata by " + + "instance: $metadataDescription. State: $serialized" + ) + } - if (event != null && event.state != null) { - val poppedState = EnroController.jsonConfiguration - .decodeFromString(event.state.toString()) - if (currentState != poppedState) { - applyNodeFor(container, poppedState) - val updatedState = createNodeFor(container) - if (updatedState != poppedState) { - window.history.back() - return@launch - } - val poppedIndex = historyStates.indexOfFirst { it == poppedState } - if (poppedIndex != -1) { - historyIndex = poppedIndex - } else { - historyStates.add(poppedState) - historyIndex = historyStates.lastIndex - } - } - } else if (event != null) { - // popstate fired without a state payload (manual address-bar edit, - // cross-origin nav). Under root-only routing we can't safely restore - // a sensible app state from URL alone — no-op and let the user - // reload if they want the URL to take effect. - return@launch - } else { // Not a popstate event (opened, active, closed, init) - val isInit = historyStates.isEmpty() && historyIndex == -1 - val isNoOp = windowState != null && windowState == currentState - - val closeIndex = historyStates.indexOfLast { it == currentState } - val isClose = closeIndex >= 0 - - when { - isInit -> { - historyStates.add(currentState) - historyIndex = 0 - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } + @OptIn(ExperimentalWasmJsInterop::class) + private suspend fun syncFromBackstack() { + val currentState = createNodeFor(rootContainer) + val serializedCurrentState = serializeForHistory(currentState).toJsString() - isNoOp -> { - val windowIndex = historyStates.indexOfLast { it == currentState } - historyIndex = windowIndex - historyStates[historyIndex] = currentState - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } + val windowState = window.history.state?.let(::decodeState) - isClose -> { - // when the target state is a close, we need to pop back to that element in the history - val previousIndex = historyIndex - historyIndex = closeIndex - historyStates[historyIndex] = currentState - val goDelta = closeIndex - previousIndex - if (closeIndex == 0) { - window.history.go(goDelta) - window.history.replaceState( - serializedCurrentState, - "example", - computeUrl(), - ) - } else { - window.history.go(goDelta - 1) - delay(1) - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - } - else -> { - val currentIndex = historyStates.indexOfLast { it == currentState } - if (currentIndex < 0) { - historyStates.subList(historyIndex + 1, historyStates.size).clear() - historyStates.add(currentState) - historyIndex = historyStates.lastIndex - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } else { - val previousIndex = historyIndex - historyIndex = currentIndex - historyStates[historyIndex] = currentState - window.history.go(previousIndex - currentIndex) - delay(1) - window.history.pushState( - serializedCurrentState, - "example", - computeUrl(), - ) - } - } + val isInit = historyStates.isEmpty() && historyIndex == -1 + val isNoOp = windowState != null && windowState == currentState + val closeIndex = historyStates.indexOfLast { it == currentState } + + when { + isInit -> { + historyStates.add(currentState) + historyIndex = 0 + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } + + isNoOp -> { + if (closeIndex >= 0) { + historyIndex = closeIndex + historyStates[historyIndex] = currentState } + window.history.replaceState(serializedCurrentState, "", computeUrl()) } - delay(1) - }.apply { - invokeOnCompletion { - activeHistoryJob = null - eventListenerEnabled = true + + closeIndex >= 0 -> { + // The current state exists earlier in the history: this is a close, + // pop back to that entry. + val previousIndex = historyIndex + historyIndex = closeIndex + historyStates[historyIndex] = currentState + if (closeIndex == 0) { + traverse(closeIndex - previousIndex) + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } else { + // Land one short of the target and push it fresh: pruning the + // browser's forward entries so forward can't resurrect screens + // the app has closed. (Not possible at index 0 — there's no + // entry before it to land on.) + traverse(closeIndex - previousIndex - 1) + window.history.pushState(serializedCurrentState, "", computeUrl()) + // The push destroyed the browser's forward entries — drop them + // from the mirror too. + historyStates.subList(historyIndex + 1, historyStates.size).clear() + } + } + + else -> { + // A state we haven't seen. Forward navigation (push) only when the + // previous state is a prefix of the new one — i.e. entries were + // added on top of what was already there. Anything else (a root + // reset such as loading → home, or a truncate-and-open section + // switch) REPLACES the current entry: the state it overwrites is + // no longer reachable in the app and must not survive as a browser + // back target. + val previous = historyStates.getOrNull(historyIndex) + val isPush = previous == null || isSubset(old = currentState, new = previous) + historyStates.subList(historyIndex + 1, historyStates.size).clear() + if (isPush) { + historyStates.add(currentState) + historyIndex = historyStates.lastIndex + window.history.pushState(serializedCurrentState, "", computeUrl()) + } else { + historyStates[historyIndex] = currentState + window.history.replaceState(serializedCurrentState, "", computeUrl()) + } } } } + + private companion object { + const val TRAVERSAL_TIMEOUT_MS = 250L + const val MAX_TRAVERSAL_ATTEMPTS = 10 + } } @@ -320,24 +472,12 @@ internal suspend fun applyNodeFor( } } -internal fun isNewState(old: ContainerNode, new: ContainerNode): Boolean { - val oldInstructions = collectInstructionIds(old).toSet() - val newInstructions = collectInstructionIds(new).toSet() - - // If the new tree has instructions not in the old tree, it's a new state - if (newInstructions.any { it !in oldInstructions }) { - return true - } - - // Check if the new tree is a subset of the old tree - return !isSubset(old, new) -} - -internal fun collectInstructionIds(node: ContainerNode): List { - val instructions = node.backstack.map { it.id } - return instructions + node.children.flatMap { collectInstructionIds(it) } -} - +/** + * True when [new] is a prefix-subset of [old] — i.e. [new] contains no entries + * that aren't already in [old], in the same order from the root. Used to + * distinguish a genuine forward push (the previous state is a subset of the + * next) from a replacement (entries were swapped out in a single transition). + */ internal fun isSubset(old: ContainerNode, new: ContainerNode): Boolean { fun isNodeSubset(oldNode: ContainerNode, newNode: ContainerNode): Boolean { if (oldNode.containerKey != newNode.containerKey) { @@ -400,4 +540,4 @@ public fun InstallWebHistoryPlugin( } ) } -} \ No newline at end of file +} diff --git a/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt b/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt new file mode 100644 index 00000000..9ad2514b --- /dev/null +++ b/enro-runtime/src/wasmJsTest/kotlin/dev/enro/test/platform/RobolectricHostTest.kt @@ -0,0 +1,3 @@ +package dev.enro.test.platform + +actual abstract class RobolectricHostTest actual constructor()