https://kotlinlang.org logo
Join Slack
Powered by
On a side note, it'd be good to have @KVisionDsl a...
# kvision
b
On a side note, it'd be good to have @KVisionDsl annotation on your builder DSLs
Copy code
@DslMarker
annotation class KVisionDsl
👍 1
r
I have noticed this annotation a few times but never had time to learn more. Looks interesting.
As far as I understand introducing this marker could break existing applications with buggy code. Should it be treated as breaking change? :-)
This is a good feature 🙂 I've already found a few bugs in one of my largest KVision app just by compiling with a snapshot. I'll add this in 4.1.0 even though it breaks some existing code.
b
Yeah, it's awesome. Limists scoping a bit and lets idea to highlight your dsls
j
Yeah! This is a great feature - I thought about suggesting it, too 😉
r
This great feature doesn't allow me to do one thing I really would like 😬. Is it possible to somehow override it for a single subclass?
b
Aren't dsl markers only available on extension functions?
r
I want to create an extension method (need two implicits receivers). It doesn't work as I would like, because DslMarker blocks one of the receivers.
b
Ah, then no, you'll have to skip those dsls
r
I can probably just get this one class (
FormPanel
) out of KVision main class tree.
or just move the tree one level down ... works like a charm 🙂
❤️ 1
t
I am upgrading to kvision 4.1 and I came to this change. I am not sure I like it. I understand the purpose of it and it's great for pure dsl, like gradle build script. But in kvision components we often need to work with properties and functions of the parent component and the code is full of explixit scope definition.
Copy code
class MyComponent: Div() {
    val button = Button("B1").apply {
        onClick { this@MyComponent.doSomething() }
    }

    lateinit var button2:Button

    init {
        div {
            buttonGroup {
                add(this@MyComponent.button)
                this@MyComponent.button2 = button("B2") {
                    onClick { this@MyComponent.doSomething() }
                }
            }
        }
    }

    fun doSomething() {}
}
r
If the problem happens to appear a lot in your own components which extend simple containers (like the one above) you can change the base class from 
Div
or 
SimplePanel
to 
BasicPanel
So this compiles fine without explicit scopes and works exactly the same:
Copy code
class MyComponent: BasicPanel() {
    val button = Button("B1").apply {
        onClick { doSomething() }
    }
    lateinit var button2: Button
    init {
        div {
            buttonGroup {
                add(button)
                button2 = button("B2") {
                    onClick { doSomething() }
                }
            }
        }
    }
    fun doSomething() {}
}
And as a side note - if you want just a simple "div" container it's probably better to extend `SimplePanel`/`BasicPanel` instead of 
Div
(which has more complicated rendering code).
t
Thank you, I didn't notice there is a BasicPanel without the dsl marker.
r
It's new in 4.1
t
We often use SimplePanel, I used a Div in the sample. Thank you.
r
If you find any other problems with DSL markers, which can't be as easily resolved, please let me know. This change can be reversed, if it will be justified enough.
t
Actually we use lot of StackPanels and TabPanels. Currently I have hundreds of places where i need to add explicit scope.
r
We could probably introduce a non-dsl marker version for all built-in containers (BasicStackPanel, BasicTabPanel etc). It would be fairly simple change in KVision code.
What do you think?
I'm not really sure. I've migrated my largest KVision apps with just a few changes.
But I mostly use plain DSL with state binding. I'm not creating a lot of my own components - I just use extension functions.
So I'll probably have to decide based on user feedback 🙂
b
Hmm, either that or I think it'd be acceptable to have all containers under different dsl markers. That way the scope would only "stop" at the next container layer
I personally prefer strict dsl scoping as i don't think implicit parent element access is easily readable nor a good practice in general
If you want something more lax and implicit, I think Typescript is a better option.
r
I've decided to play a bit with this topic. I've created non-dsl versions of all build-in containers from the 
i.k.panel
package. And I've marked them as experimental with new 
@RequiresOptIn
annotation. So everyone will see the message when they try to use them.
👍 1
b
Just be sure to put a section in the docs explaining how to add that annotation to the entire sourceSet via gradle buildscript or you'll end up getting exact same issues from people 😀
r
I have no idea how to do this 🙂
b
t
I agree. I also prefer strict rules/types/constrains but in this case I don't think it help us. We don't write long nested dsl blocks but we often write custom components (extending existing ones) and access defining component from the dsl (usually in event handlers). Then we compose the UI from these components and reuse them in other places. Adding Basic* containers did not help, because we often extend other components (Modal, ToolBar, Nav). Currently I have ~500 places where to add explicit scope definition.
It requires explicit scope even in apply function, we quite often need to create components pragmatically
Copy code
val mainComponent = TabPanel(classes = setOf("panel-view"), scrollableTabs = true).apply {
        height = this@DialogueSourceEditor.mainDefaultHeight.px
        addAll(listOf(
            this@DialogueSourceEditor.graphEditorTab,
            this@DialogueSourceEditor.initCodeEditorTab,
            this@DialogueSourceEditor.propertiesTab,
            this@DialogueSourceEditor.descriptionTab,
            this@DialogueSourceEditor.sourceTab,
            this@DialogueSourceEditor.workflowTab
        ))
    }
b
@Robert Jaros I know this would be duplicated effort, but would it make sense then to extract all DSLs into two separate modules containing exact same code, but one not having @DslMarker annotations? The end-user could then pick one of those
j
Honestly I don't get the problem yet. @DslMarker should be very helpful and avoid wrong calls. If it is necessary/helpful to call methods of parents, maybe these methods shouldn't be annotated at all?
b
I think not having @DslMarker on all containers, but keeping it on all components would make most sense.
r
I think we can consider making a step back. I understand Tomas's issue. KVision is OO in the first place. The DslMarker is just a helper - it should not break other things.
My idea is to remove DslMarker from all top level containers, drop Basic* containers and add DslMarker to the 
Tag
class in 
html
package. This way it will be applied only to typical html components and strictly html-like markup code. It should still catch some errors and not break anything like it currently does.
What do you think?
t
I would be happy with it:)
r
@Tomas Kormanak I've pushed these changes to master. You can build your local version and test your app.
t
Thank you!
Open in Slack
PreviousNext
Powered by