[Development] QTCS2019 Notes from QtQml session

[Robin Burchell]

On Mon, Nov 25, 2019, at 10:43 AM, Ulf Hermann wrote:
> Indeed we need to deal with this somehow. Maybe we do need to retain 
> versioning on a module level. For example, the qmldir file could specify 
> what versions of which other modules it expects. Or we define that it's 
> not a problem ...

I think at least module-level versioning is a sensible idea.

> We could detect this in qmllint by just continuing the search for IDs
> even if we've found a property.

We could, but if I may gently suggest, I think I can count most of the people who use qmllint on a regular, useful basis on one hand ;-).

(One possible improvement to this could be to not _just_ have it as a separate tool: maybe debug builds should _also_ run qmllint - or at least a subset of its checks - on the code that the engine is loading? Alternatively, perhaps as an engine flag to at least allow it to be turned on once, and happen for everyone on a team in a unified zero-effort way)

> Mind that adding a method to a base class in C++ will shadow unrelated 
> other classes and functions even if it's not virtual or overridden. Why 
> did we actually decide that we can live with this effect in C++ but not 
> in QML? - Maybe because in C++ it's more likely to result in a compile 
> error due to mismatched signatures?

I think one of the biggest problems is that ID resolution crosses file boundaries. This essentially means that the ids chosen can very easily become part of the "API" of a component unless you are very vigilant to not allow that to happen.

Essentially this means that identifiers - and a child redefining an identifier that the parent happened to use - suffer from the same problem that you point out class members can suffer from.

This is a contrived example, but I have seen similar things to this cause bugs in production.

Picture a SettingsButton.qml with this sort of contents:
    Button { MouseArea { onClicked: console.log("clicked"); } }

Looks nice and innocent, right? But I notice that the MouseArea doesn't work because it has no geometry. I want to refer to it elsewhere in this component, too, so let's give it a name.

    Button { MouseArea { id: someFoo; width: parent.width; onClicked: console.log("clicked"); } }

In sensible code, this would work just fine, and have no complications.
Unfortunately, what you didn't know was that Button.qml is:

    Rectangle {
        width: someFoo.width
        height: 10
    }

The use of someFoo here was previously referring to a parent id. Now, it finds that id in the child instead, with a very different geometry to what was expected - just the same as if this was a property on the root item.

This is not an easy problem I think. For sure, you can't just take the "easy way" out and warn about duplicate ids during lookup, because while _sometimes_ this inheritance is decidedly wrong and not what you want, a lot of the time, id scoping is "private" and just fundamentally how code is written. In the C++ world, this would seem pretty silly:

    class Base { private: int m_myImplementationDetail; }
    class Base : Derived { private: int m_myImplementationDetail; } // warning: duplicate variable "m_myImplementationDetail" found in class Base

Yes, they do indeed have the same name, but that's also unambiguous and fine.

If we do consider them to be part of the API *sometimes* though, perhaps the right thing to do is to allow that to happen explicitly: specify what ids can be "exported/imported" and issue a compile error if those expectations are not met.

One problem though is that this import/export boundary should be checked at the file boundary, _not_ the component boundary, otherwise using things like Loader become a lot more annoying.

So perhaps it could look something like this:

import MyModule ...
expects Item myFoo; // we want something of type Item, identified as myFoo to be passed in from a containing file somewhere
provides MouseArea bar; // we provide this ID for children to find

Item {
    width: myFoo.width

    MouseArea {
        id: bar // is visible in scoped lookup (via 'provides' at the top)
    }
}

I guess you might also be able to use this for context properties - which suffer from many of the same unclear scoping issues rather than just outright removing them, but that might require some additional considerations that I'm not sure of right now.

-- 
  Robin Burchell
  robin at crimson.no