KTOR-7584 Moved experimental htmx annotation to a common type #4883
Conversation
WalkthroughThis change removes the Changes
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxHeaders.kt (1)
12-47: 💡 Verification agent🧩 Analysis chain
Consistent annotation migration across the htmx module.
The annotation changes from
@ExperimentalHtmxApito@ExperimentalKtorApiare correctly applied to all public APIs.Since this is a breaking change for consumers who opt-in to these experimental APIs, let me verify the migration impact:
🏁 Script executed:
#!/bin/bash # Description: Check for documentation and test updates related to this annotation change # Test: Look for README or documentation updates mentioning the annotation change fd -e md -e adoc | xargs grep -l -E "(ExperimentalHtmxApi|ExperimentalKtorApi|htmx.*experimental)" # Test: Check if tests have been updated to use the new annotation rg -A 2 "@OptIn\(ExperimentalHtmxApi::class\)" # Test: Look for migration guides or CHANGELOG entries fd -i -e md -e txt "changelog|migration|breaking" | xargs grep -l -i "htmx\|experimental"Length of output: 254
🌐 Web query:
Is there a standard practice in Ktor for migrating experimental API annotations? What should consumers do when experimental annotations change?💡 Result:
Yes, in Ktor (and Kotlin in general), there is a standard practice for handling experimental API annotations and their migration.
Context
Ktor uses Kotlin's experimental feature mechanism with annotations such as
@ExperimentalKtorApi. When an API is marked experimental, it means its behavior, signature, or existence might change in the future. This requires consumers of such APIs to explicitly opt-in by using annotations like@OptIn(ExperimentalKtorApi::class).Migrating Experimental Annotations
When experimental API annotations change (for example, if they are renamed, replaced, or removed), Ktor encourages the following approach:
- Deprecate the old experimental annotation:
- The old annotation is usually marked with
@Deprecatedto warn users.- Introduce a new experimental annotation:
- This new annotation often covers updated contracts or semantics.
- Consumers must update their opt-in annotations:
- Clients need to replace the old opt-in annotations with the new ones to continue using the experimental APIs without warnings.
What Consumers Should Do
- Track changes in Ktor versions and their experimental annotations. Keep an eye on release notes or migration guides.
- Update opt-in annotations in your code. For example, if the old annotation was
@OptIn(OldExperimentalApi::class), and it changes to@OptIn(NewExperimentalApi::class), update your source accordingly.- Refactor as needed based on the new API semantics. The API itself may have evolved, so adapting your code might be necessary.
- Optionally, keep code backward compatible if supporting multiple Ktor versions, using conditional compilation or separate branches.
Summary
There's no automatic migration for experimental annotations; consumers must manually update their code on annotation changes to align with Ktor’s evolving experimental APIs. This explicit opt-in approach is intentional to make developers aware of and consent to using unstable or changing APIs.
If you'd like, I can help find specific examples or migration notes from recent Ktor versions to illustrate this better. Would you like me to do that?
🏁 Script executed:
#!/bin/bash # Inspect CHANGELOG.md for entries related to htmx experimental annotation migration grep -n -C2 -E "(ExperimentalHtmxApi|ExperimentalKtorApi|htmx)" CHANGELOG.mdLength of output: 78
Add migration documentation for the experimental annotation change
I’ve verified that the code correctly replaces
@ExperimentalHtmxApiwith@ExperimentalKtorApiacross the htmx module, but I did not find any corresponding updates to guide consumers through this breaking change:• No entries in CHANGELOG.md covering the annotation migration
• No updates in README or other documentation files
• No deprecation warning on the oldExperimentalHtmxApiannotationPlease add:
- A CHANGELOG.md entry noting that htmx APIs moved from
@ExperimentalHtmxApito@ExperimentalKtorApi- Migration instructions in your documentation (README or a dedicated migration guide)
- (Optional) Mark the old annotation as deprecated to aid consumer transition
🧹 Nitpick comments (1)
ktor-shared/ktor-htmx/ktor-htmx-html/common/test/io/ktor/htmx/html/HxAttributesTest.kt (1)
7-8: Consider using explicit imports instead of wildcards.While the wildcard imports work correctly, using explicit imports would be more maintainable and clearer about dependencies.
-import io.ktor.* -import io.ktor.htmx.* +import io.ktor.ExperimentalKtorApi +import io.ktor.htmx.HxSwap
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxHeaders.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/test/io/ktor/server/htmx/HxRoutingTest.kt(2 hunks)ktor-shared/ktor-htmx/api/ktor-htmx.api(0 hunks)ktor-shared/ktor-htmx/api/ktor-htmx.klib.api(0 hunks)ktor-shared/ktor-htmx/common/src/io/ktor/htmx/Annotations.kt(0 hunks)ktor-shared/ktor-htmx/ktor-htmx-html/common/src/io/ktor/htmx/html/HxAttributes.kt(1 hunks)ktor-shared/ktor-htmx/ktor-htmx-html/common/test/io/ktor/htmx/html/HxAttributesTest.kt(2 hunks)ktor-utils/api/ktor-utils.api(1 hunks)ktor-utils/api/ktor-utils.klib.api(1 hunks)ktor-utils/common/src/io/ktor/Annotations.kt(1 hunks)
💤 Files with no reviewable changes (3)
- ktor-shared/ktor-htmx/common/src/io/ktor/htmx/Annotations.kt
- ktor-shared/ktor-htmx/api/ktor-htmx.api
- ktor-shared/ktor-htmx/api/ktor-htmx.klib.api
🔇 Additional comments (10)
ktor-utils/common/src/io/ktor/Annotations.kt (1)
7-20: Well-structured experimental API annotation.The annotation is properly defined with all necessary meta-annotations (
@RequiresOptIn,@MustBeDocumented,@Retention(AnnotationRetention.BINARY)) and clear documentation about its purpose and usage. This provides a good foundation for marking experimental APIs across the Ktor codebase.ktor-utils/api/ktor-utils.klib.api (1)
10-12: API definition correctly reflects the new annotation.The Klib API dump properly documents the new
ExperimentalKtorApiannotation with the correct signature and inheritance.ktor-utils/api/ktor-utils.api (1)
1-3: LGTM! Clean addition of the new experimental API marker.The new
ExperimentalKtorApiannotation interface is properly declared and aligns well with the PR objective of consolidating experimental API markers into a common type.ktor-server/ktor-server-plugins/ktor-server-htmx/common/test/io/ktor/server/htmx/HxRoutingTest.kt (1)
7-7: Correctly updated to use the new experimental API marker.The import and @OptIn annotation have been properly updated to use
ExperimentalKtorApiinstead ofExperimentalHtmxApi, maintaining consistency with the refactoring.Also applies to: 22-22
ktor-shared/ktor-htmx/ktor-htmx-html/common/test/io/ktor/htmx/html/HxAttributesTest.kt (1)
18-18: Correctly updated to use the new experimental API marker.The @OptIn annotation has been properly updated to use
ExperimentalKtorApi.ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt (2)
7-7: Import correctly added for the new experimental API marker.
16-16: All experimental annotations properly updated.The
@ExperimentalKtorApiannotation has been correctly applied to all public API elements (hxproperty,hxfunction, andHxRouteclass), maintaining the experimental status while using the new common marker.Also applies to: 22-22, 32-32
ktor-shared/ktor-htmx/ktor-htmx-html/common/src/io/ktor/htmx/html/HxAttributes.kt (2)
7-7: LGTM!The import for
ExperimentalKtorApiis correctly added to support the annotation changes.
15-24:✅ Verification successful
Annotation migration looks good.
The consistent replacement of
@ExperimentalHtmxApiwith@ExperimentalKtorApialigns with the PR objectives. This consolidation will help avoid future API breakage.Let me verify that all references to the old annotation have been updated:
🏁 Script executed:
#!/bin/bash # Description: Verify that ExperimentalHtmxApi annotation is completely removed from the codebase # Test: Search for any remaining usage of ExperimentalHtmxApi rg -A 2 "ExperimentalHtmxApi" # Test: Check if the old annotation class definition still exists fd -e kt -e java --exec grep -l "annotation class ExperimentalHtmxApi" {} \;Length of output: 111
Annotation migration complete
- Verified with
rgandfdthat there are no remaining occurrences or definitions ofExperimentalHtmxApiin the codebase.All references have been successfully consolidated under
@ExperimentalKtorApi. No further changes are needed.ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxHeaders.kt (1)
3-3: LGTM!The import is correctly added to support the annotation migration.
| @RequiresOptIn | ||
| @MustBeDocumented | ||
| @Retention(AnnotationRetention.BINARY) | ||
| public annotation class ExperimentalKtorApi |
There was a problem hiding this comment.
Btw, we have deprecated io.ktor.utils.io.KtorExperimentalAPI. Probably we should do something with it. Maybe change its deprecation level to HIDDEN?
There was a problem hiding this comment.
Oh, and it would be nice to move InternalAPI here and rename it to InternalKtorApi, but it is definitely out of the PR's scope
There was a problem hiding this comment.
Oh, yeah that makes a lot of sense. I didn't know we had a KtorExperimentalAPI already what the heck I wonder why it was deprecated.
There was a problem hiding this comment.
I only found this: KTOR-2204. But it gives no details
There was a problem hiding this comment.
We decided to avoid using the experimental API naming because of the confusing contract. It's not clear what we mean by experimental. I would avoid using Experimental naming and try clarifying what the consequences of using such an API are.
There was a problem hiding this comment.
I think I'll just un-deprecate the previous annotation instead of introducing the new one. I've created a ticket for renaming the @InternalApi for the next major release so we don't forget.
There was a problem hiding this comment.
Could you explain the reason and how people should decide to opt in this annotation?
There was a problem hiding this comment.
Yeah, my thinking is that we need some user feedback on the HTMX additions before committing to the API since it's a fairly subjective and unfamiliar framework / workflow. I think anyone interested in Ktor/HTMX ought to try it out since it's not normally used for big applications and future changes should be pretty minor.
bjhham
force-pushed
the
bjhham/hx-experimental-flag
branch
from
8fa268d to
5206cb1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
ktor-io/api/ktor-io.api(1 hunks)ktor-io/api/ktor-io.klib.api(1 hunks)ktor-io/common/src/io/ktor/utils/io/Annotations.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxHeaders.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/test/io/ktor/server/htmx/HxRoutingTest.kt(1 hunks)ktor-shared/ktor-htmx/api/ktor-htmx.api(0 hunks)ktor-shared/ktor-htmx/api/ktor-htmx.klib.api(0 hunks)ktor-shared/ktor-htmx/common/src/io/ktor/htmx/Annotations.kt(0 hunks)ktor-shared/ktor-htmx/ktor-htmx-html/common/src/io/ktor/htmx/html/HxAttributes.kt(1 hunks)ktor-shared/ktor-htmx/ktor-htmx-html/common/test/io/ktor/htmx/html/HxAttributesTest.kt(2 hunks)
💤 Files with no reviewable changes (3)
- ktor-shared/ktor-htmx/common/src/io/ktor/htmx/Annotations.kt
- ktor-shared/ktor-htmx/api/ktor-htmx.api
- ktor-shared/ktor-htmx/api/ktor-htmx.klib.api
✅ Files skipped from review due to trivial changes (3)
- ktor-shared/ktor-htmx/ktor-htmx-html/common/test/io/ktor/htmx/html/HxAttributesTest.kt
- ktor-io/api/ktor-io.klib.api
- ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxHeaders.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- ktor-server/ktor-server-plugins/ktor-server-htmx/common/test/io/ktor/server/htmx/HxRoutingTest.kt
- ktor-shared/ktor-htmx/ktor-htmx-html/common/src/io/ktor/htmx/html/HxAttributes.kt
- ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt
🔇 Additional comments (2)
ktor-io/api/ktor-io.api (1)
266-267: LGTM! Standard annotation API declaration.The addition of
ExperimentalKtorApito the public API follows the expected pattern for annotation interfaces in the JVM bytecode representation.ktor-io/common/src/io/ktor/utils/io/Annotations.kt (1)
28-28: Good improvement: Adding binary retention to InternalAPI.Adding
@Retention(AnnotationRetention.BINARY)ensures the annotation is preserved in compiled bytecode, which is important for tooling and runtime inspection.
| /** | ||
| * This API may change in a future release depending on feedback. | ||
| * | ||
| * If you'd like to provide some feedback regarding the experimental API, | ||
| * please reach out to us via one of the channels listed here: [https://ktor.io/support/](https://ktor.io/support/) | ||
| * | ||
| * Any usage of a declaration annotated with `@ExperimentalKtorApi` must be accepted either by | ||
| * annotating that usage with the [OptIn] annotation, e.g. `@OptIn(ExperimentalStdlibApi::class)`, | ||
| * or by using the compiler argument `-opt-in=kotlin.ExperimentalKtorApi`. | ||
| */ | ||
| @RequiresOptIn( | ||
| level = RequiresOptIn.Level.WARNING, | ||
| message = "This API is experimental. " + | ||
| "It could be removed or changed in future releases, or its behaviour may be different." | ||
| ) | ||
| @MustBeDocumented | ||
| @Retention(AnnotationRetention.BINARY) | ||
| public annotation class ExperimentalKtorApi |
There was a problem hiding this comment.
Fix documentation inaccuracies in the new annotation.
The ExperimentalKtorApi annotation is well-structured, but the documentation contains inaccuracies that should be corrected:
- Line 62: References the wrong annotation class in the example
- Line 63: Uses incorrect package name in the compiler argument
Apply this diff to fix the documentation:
- * annotating that usage with the [OptIn] annotation, e.g. `@OptIn(ExperimentalStdlibApi::class)`,
- * or by using the compiler argument `-opt-in=kotlin.ExperimentalKtorApi`.
+ * annotating that usage with the [OptIn] annotation, e.g. `@OptIn(ExperimentalKtorApi::class)`,
+ * or by using the compiler argument `-opt-in=io.ktor.utils.io.ExperimentalKtorApi`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * This API may change in a future release depending on feedback. | |
| * | |
| * If you'd like to provide some feedback regarding the experimental API, | |
| * please reach out to us via one of the channels listed here: [https://ktor.io/support/](https://ktor.io/support/) | |
| * | |
| * Any usage of a declaration annotated with `@ExperimentalKtorApi` must be accepted either by | |
| * annotating that usage with the [OptIn] annotation, e.g. `@OptIn(ExperimentalStdlibApi::class)`, | |
| * or by using the compiler argument `-opt-in=kotlin.ExperimentalKtorApi`. | |
| */ | |
| @RequiresOptIn( | |
| level = RequiresOptIn.Level.WARNING, | |
| message = "This API is experimental. " + | |
| "It could be removed or changed in future releases, or its behaviour may be different." | |
| ) | |
| @MustBeDocumented | |
| @Retention(AnnotationRetention.BINARY) | |
| public annotation class ExperimentalKtorApi | |
| /** | |
| * This API may change in a future release depending on feedback. | |
| * | |
| * If you'd like to provide some feedback regarding the experimental API, | |
| * please reach out to us via one of the channels listed here: [https://ktor.io/support/](https://ktor.io/support/) | |
| * | |
| * Any usage of a declaration annotated with `@ExperimentalKtorApi` must be accepted either by | |
| * annotating that usage with the [OptIn] annotation, e.g. `@OptIn(ExperimentalKtorApi::class)`, | |
| * or by using the compiler argument `-opt-in=io.ktor.utils.io.ExperimentalKtorApi`. | |
| */ | |
| @RequiresOptIn( | |
| level = RequiresOptIn.Level.WARNING, | |
| message = "This API is experimental. " + | |
| "It could be removed or changed in future releases, or its behaviour may be different." | |
| ) | |
| @MustBeDocumented | |
| @Retention(AnnotationRetention.BINARY) | |
| public annotation class ExperimentalKtorApi |
🤖 Prompt for AI Agents
In ktor-io/common/src/io/ktor/utils/io/Annotations.kt around lines 55 to 72, the
documentation for the ExperimentalKtorApi annotation incorrectly references the
OptIn example annotation class and the compiler argument package name. Update
the example annotation from ExperimentalStdlibApi to ExperimentalKtorApi and
correct the compiler argument to use the proper package name for
ExperimentalKtorApi to ensure accurate guidance for users opting into this
experimental API.
Subsystem
Shared, HTMX
Motivation
KTOR-7584 First-class HTMX support
I realized having a specific flag doesn't make sense since we can't remove it without breaking the API.
Solution
Moved the annotation to a common type so we can mark other things as such when we want to solicit some feedback before committing to a stable API.