bug in a JS function

mzimmers

Hi all -

I've written a callback function that has a bug in it. I've traced it to a single line, but can't see what's wrong with it. I apologize for the verbosity of the variable names.

console.log("EquipmentSelectValves.qml.replaceValve(): i is " + i)
console.log("EquipmentSelectValves.qml.replaceValve(): unassignedValves is " + unassignedValves)

var pumpUuid = equipmentCopy.uuid
console.log("EquipmentSelectValves.qml.replaceValve(): pumpUuid is " + pumpUuid)

var assignedValves = []
console.log("EquipmentSelectValves.qml.replaceValve(): assignedValves is an array: " + Array.isArray(assignedValves))

assignedValves = outcome.valveConfigurationList
console.log("EquipmentSelectValves.qml.replaceValve(): assignedValves size is " + assignedValves.length)
console.log("EquipmentSelectValves.qml.replaceValve(): assignedValves (before) is " + assignedValves)

var indexToReplace = assignedValves.indexOf(modelData)
console.log("EquipmentSelectValves.qml.replaceValve(): indexToReplace is " + indexToReplace)

var newValveUuid = unassignedValves[i]
console.log("EquipmentSelectValves.qml.replaceValve(): newValveUuid is " + newValveUuid)

assignedValves[indexToReplace] = newValveUuid // THIS LINE IS CAUSING THE PROBLEM.
console.log("EquipmentSelectValves.qml.replaceValve(): assignedValves (after) is " + assignedValves)

outcome.valveConfigurationList = assignedValves
console.log("EquipmentSelectValves.qml.replaceValve(): outcome.valveConfigurationList is " + outcome.valveConfigurationList) // THIS IS THE LINE WITH THE ERROR BELOW.

Here's the console output:

qml: EquipmentSelectValves.qml.replaceValve(): i is 0
qml: EquipmentSelectValves.qml.replaceValve(): unassignedValves is {fb4f3c69-312e-4399-8d80-ffabe2deab9f},{fe93c695-7f7c-4af5-8ff8-8010fa6db546}
qml: EquipmentSelectValves.qml.replaceValve(): pumpUuid is {a277e2f8-5f30-4197-8092-d6f3c345f85d}
qml: EquipmentSelectValves.qml.replaceValve(): assignedValves is an array: true
qml: EquipmentSelectValves.qml.replaceValve(): assignedValves size is 2
qml: EquipmentSelectValves.qml.replaceValve(): assignedValves (before) is {821856d4-deb1-4b55-9ba2-0fc4fee11ad2},{68dbce5b-c0c3-4f95-a9f5-7db9f0351421}
qml: EquipmentSelectValves.qml.replaceValve(): indexToReplace is 1
qml: EquipmentSelectValves.qml.replaceValve(): newValveUuid is {fb4f3c69-312e-4399-8d80-ffabe2deab9f}
qml: EquipmentSelectValves.qml.replaceValve(): assignedValves (after) is {821856d4-deb1-4b55-9ba2-0fc4fee11ad2},{fb4f3c69-312e-4399-8d80-ffabe2deab9f}
qrc:/qt/qml/NgaIcdFw/qml/equipment/EquipmentSelectValves.qml:202: ReferenceError: outcome is not defined

It's as though something in my routine is clobbering itself. Can anyone see something I'm doing wrong here?

Thanks...

JKSH

Looks like you've found an interesting bug related to Components accessing external IDs.

Add pragma ComponentBehavior: Bound to the top of EquipmentSelectValves.qml (see https://doc.qt.io/qt-6/qtqml-documents-structure.html#componentbehavior ). This makes the QML engine a bit stricter though, so you need to also mark your delegate's modelData and index properties as required property (see the examples at https://doc.qt.io/qt-6/qtquick-modelviewsdata-modelview.html#models ). This is not necessarily a bad thing, as it enforces good practice.

Note: After this, your example will no longer complain that outcome is not defined, but it will complain about equipmentModel because that is truly not defined anywhere in your example.

mzimmers

I think I found the problem, but I don't understand it. Here's a simplified snippet:

ListView {
    id: valveList
    model: outcome.valveConfigurationList
    delegate: valveBox
    Component {
        id: valveBox
        TextIconButton {
            id: textIconButton
            onClicked: {
                valvesSideDrawer.open()
                valvesDrawerView.push("components/EquipmentDrawer.qml",
                                      {
                                          listModel: unassignedValves,
                                          titleText: "Valves",
                                          callback: replaceValve
                                      })
            }

            function replaceValve(b, i) {
                outcome.valveConfigurationList = assignedValves
                console.log("EquipmentSelectValves.qml.replaceValve(): outcome.valveConfigurationList is " + outcome.valveConfigurationList)

Note that I'm trying to modify the model (outcome.valveConfigurationList) from within the delegate. Should this be permitted? The C++ function seems to work correctly when this happens, but I'm wondering if I'm somehow sending the QML engine into the weeds...

JKSH

@mzimmers How is outcome defined?

mzimmers

@JKSH

class Outcome : public QObject
{
    Q_OBJECT
    QML_ELEMENT
    ...
    Q_PROPERTY(ValveConfigurationList valveConfigurationList READ valveConfigurationList WRITE setValveConfigurationList NOTIFY valveConfigurationListChanged FINAL)

And the instance I'm using is in a parent QML file:

property var outcome: outcomeComponent.createObject(parent)

    Component {
        id: outcomeComponent
        Outcome {
            id: newOutcome
        }
    }

JKSH

@mzimmers said in bug in a JS function:

And the instance I'm using is in a parent QML file:

Make your variables fully-qualified (see https://forum.qt.io/topic/156292/how-to-order-object-creation/ ). That should also take care of your ReferenceError

mzimmers

@JKSH thanks for the suggestion. Unfortunately, I'm still having the same problem, except now it's equipmentEdit (see below) that's undefined.

// EquipmentConfigEdit.qml
ColumnLayout {
    id: equipmentEdit
    property var outcome: outcomeComponent.createObject(equipmentEdit) as Outcome

    Component {
        id: outcomeComponent
        Outcome {
            id: newOutcome
        }
    }

Used here (I've removed most of the logs):

function replaceValve(b, i) {
    if (b === 2) { // and it is 2
        var pumpUuid = equipmentEdit.equipmentCopy.uuid
        var assignedValves = equipmentEdit.outcome.valveConfigurationList
        var indexToReplace = assignedValves.indexOf(modelData)
        var newValveUuid = unassignedValves[i]
        assignedValves[indexToReplace] = newValveUuid
        assignedValves.splice(indexToReplace, 1, newValveUuid)

 // this line causes the trouble.
        equipmentEdit.outcome.valveConfigurationList = assignedValves
        
        console.log("EquipmentSelectValves.qml.replaceValve(): equipmentEdit.outcome.valveConfigurationList is " + equipmentEdit.outcome.valveConfigurationList)

All my logs suggest that everything is working...up to my assignment of the valve configuration list. I should point out that it isn't just outcome (or equipmentEdit) that is now undefined; it appears to be anything that comes after that statement.

JKSH

@mzimmers It's not clear from your snippets:

Where is replaceValve() defined? Is it inside EquipmentConfigEdit.qml?
Where is `unassignedValves defined? What does it contain?
What is the definition of ValveConfigurationList?

mzimmers

@JKSH said in bug in a JS function:

@mzimmers It's not clear from your snippets:

Where is replaceValve() defined? Is it inside EquipmentConfigEdit.qml?

No, it's in another QML file. EquipmentConfigEdit.qml has a DelegateChoice that uses VspEdit.qml, which uses another file with the above script.

Where is `unassignedValves defined? What does it contain?

In EquipmentConfigEdit.qml (I forgot to qualify that one; now fixed). Here's the definition:

property var unassignedValves: outcomeModel.unassignedValves()

What is the definition of ValveConfigurationList?

#define ValveConfigurationList QList<QUuid>
class Outcome: public QObject
{
    Q_OBJECT
    QML_ELEMENT
    Q_PROPERTY(ValveConfigurationList valveConfigurationList READ valveConfigurationList WRITE setValveConfigurationList NOTIFY valveConfigurationListChanged FINAL)
    ValveConfigurationList valveConfigurationList() { return m_valveConfigurationList; }
    bool setValveConfigurationList(const ValveConfigurationList &list);

JKSH

@mzimmers said in bug in a JS function:

#define ValveConfigurationList QList<QUuid>
class Outcome: public QObject
{
    Q_OBJECT
    QML_ELEMENT
    Q_PROPERTY(ValveConfigurationList valveConfigurationList READ valveConfigurationList WRITE setValveConfigurationList NOTIFY valveConfigurationListChanged FINAL)
    ValveConfigurationList valveConfigurationList() { return m_valveConfigurationList; }
    bool setValveConfigurationList(const ValveConfigurationList &list);

Hmm... I'm not sure if the #define will play nicely with Qt's property system. https://doc.qt.io/qt-6/qtqml-cppintegration-exposecppattributes.html#exposing-properties says "Do not use typedef or using for Q_PROPERTY types as these will confuse moc). I'd imagine that the same goes for #define.

As a sanity check, put qDebug() << list; inside setValveConfigurationList(). Is your C++ function receiving the expected list?

Try replacing ValveConfigurationList with QList<QUuid> throughout your header. Does that make a difference? (If not, your best bet for quick troubleshooting is to provide a minimal, compilable example that triggers the issue)

Where is replaceValve() defined? Is it inside EquipmentConfigEdit.qml?

No, it's in another QML file. EquipmentConfigEdit.qml has a DelegateChoice that uses VspEdit.qml, which uses another file with the above script.

OK. And does VspEdit contain required property EquipmentConfigEdit equipmentEdit or similar? (Remember, use EquipmentConfigEdit instead of var)

Where is `unassignedValves defined? What does it contain?

In EquipmentConfigEdit.qml (I forgot to qualify that one; now fixed). Here's the definition:
property var unassignedValves: outcomeModel.unassignedValves() 

I don't think this is related to your issue, but do you need a separate property called EquipmentConfigEdit.unassignedValves? Could you just do let newValveUuid = equipmentEdit.outcomeModel.unassignedValves[i] or similar?

mzimmers

@JKSH said in bug in a JS function:

Hmm... I'm not sure if the #define will play nicely with Qt's property system. https://doc.qt.io/qt-6/qtqml-cppintegration-exposecppattributes.html#exposing-properties says "Do not use typedef or using for Q_PROPERTY types as these will confuse moc). I'd imagine that the same goes for #define.
As a sanity check, put qDebug() << list; inside setValveConfigurationList(). Is your C++ function receiving the expected list?

I stepped into it in the debugger; it seems to be getting the correct list.

Try replacing ValveConfigurationList with QList<QUuid> throughout your header. Does that make a difference? (If not, your best bet for quick troubleshooting is to provide a minimal, compilable example that triggers the issue)

No, it doesn't.

Where is replaceValve() defined? Is it inside EquipmentConfigEdit.qml?

No, it's in another QML file. EquipmentConfigEdit.qml has a DelegateChoice that uses VspEdit.qml, which uses another file with the above script.

OK. And does VspEdit contain required property EquipmentConfigEdit equipmentEdit or similar? (Remember, use EquipmentConfigEdit instead of var)

No. VspEdit is used within EquipmentConfigEdit.qml as folllows:

ListView {
    model: equipmentCopy
    delegate: DelegateChooser {
        role: "category"
        DelegateChoice {
            roleValue: NgaUI.CATEGORY_VSP
            delegate: VspEdit {
                equipment: equipmentEdit.equipment
                equipmentCopy: equipmentEdit.equipmentCopy

equipmentEdit is the id assigned to EquipmentConfigEdit.qml.

Where is `unassignedValves defined? What does it contain?
Also in EquipmentConfigEdit.qml:

property var unassignedValves: outcomeModel.unassignedValves()

I don't think this is related to your issue, but do you need a separate property called EquipmentConfigEdit.unassignedValves? Could you just do let newValveUuid = equipmentEdit.outcomeModel.unassignedValves[i] or similar?

I did this (no change in results):

var newValveUuid = outcomeModel.unassignedValves()[i]

I will work on a minimal example. Thanks for the help.

mzimmers

First attempt at a minimal example produced a working script (crud).

I'm 99% sure that I'm successfully accessing the variables created in the file EquipmentConfigEdit.qml. Now, with the fully qualified variable names, I'm getting the same error, but on "equipmentEdit."

assignedValves[indexToReplace] = newValveUuid
console.log("EquipmentSelectValves.qml.replaceValve(): assignedValves (after) is " + assignedValves)

equipmentEdit.outcome.valveConfigurationList = assignedValves
// line 197 below.
console.log("EquipmentSelectValves.qml.replaceValve(): equipmentEdit.outcome.valveConfigurationList is " + equipmentEdit.outcome.valveConfigurationList)

produces this error:

qrc:/qt/qml/NgaIcdFw/qml/equipment/EquipmentSelectValves.qml:197: ReferenceError: equipmentEdit is not defined

if I comment out that log, the next variable I hit is said to be not defined.

It's really like the call to setValveConfigurationList() is somehow clobbering something in my script.

mzimmers

@JKSH I have a minimal example now. I've zipped it up. Where would you like me to put it?

JKSH

@mzimmers said in bug in a JS function:

@JKSH I have a minimal example now. I've zipped it up. Where would you like me to put it?

Any file sharing service of your choice should be fine (Dropbox, Google Drive, Microsoft OneDrive, etc.) Or you could push it to a public Git host (e.g. GitHub, Atlassian Bitbucket, etc.)

mzimmers

@JKSH Here it is:

my zip zile

If you can figure out what's going on with this, I'll buy you a seafood dinner the next time you're in Monterey.

JKSH

Looks like you've found an interesting bug related to Components accessing external IDs.

Add pragma ComponentBehavior: Bound to the top of EquipmentSelectValves.qml (see https://doc.qt.io/qt-6/qtqml-documents-structure.html#componentbehavior ). This makes the QML engine a bit stricter though, so you need to also mark your delegate's modelData and index properties as required property (see the examples at https://doc.qt.io/qt-6/qtquick-modelviewsdata-modelview.html#models ). This is not necessarily a bad thing, as it enforces good practice.

Note: After this, your example will no longer complain that outcome is not defined, but it will complain about equipmentModel because that is truly not defined anywhere in your example.

mzimmers

@JKSH said in bug in a JS function:

Looks like you've found an interesting bug related to Components accessing external IDs.

I love when things aren't (entirely) my fault.

Add pragma ComponentBehavior: Bound to the top of EquipmentSelectValves.qml
you need to also mark your delegate's modelData and index properties as required property

Done and done.

it will complain about equipmentModel because that is truly not defined anywhere in your example.

Yeah, I forgot to remove that part (because it bombed before it got there). Shouldn't be a problem in my real app, where I have this line in main.cpp:

    engine.rootContext()->setContextProperty("equipmentModel", equipmentModel);

Let me know when you're in town, and I'll buy you the best dinner Monterey can offer (which is pretty damn good, IMO).

Should I report this bug, and if so, how should I characterize it?

JKSH

Actually, I don't think it's a bug after all. Basically:

Your replaceValve() function is a "member" of your delegate. (More precisely, replaceValve() exists within the context of your valveBox delegate, which is separate from the context of EquipmentSelectValves by default)
That function modifies the ListView's model.
Modifying the model caused your delegate (and its context) to be destroyed.
Destroying the delegate's context invalidates the references inside replaceValve().

Here is a minimal reproducer for your issue:

//pragma ComponentBehavior: Bound
import QtQuick
import QtQuick.Controls.Basic

Window {
    width: 800
    height: 600
    visible: true

    ListView {
        id: lv
        anchors.fill: parent
        model: 1
        delegate: Button {
            text: "Click me"

            onClicked: {
                console.log("parent: " + parent)
                console.log("lv: " + lv)
                lv.model = 0
                console.log("parent: " + parent)
                console.log("lv: " + lv)
            }

            Component.onDestruction: console.log("Destroyed", this)
        }
    }
}

Output:

qml: parent: QQuickItem(0x1257086c4a0)
qml: lv: QQuickListView(0x12570653350)
qml: Destroyed Button_QMLTYPE_1(0x12570653b30)
qml: parent: null
Main.qml:22: ReferenceError: lv is not defined

pragma ComponentBehavior: Bound changes QML's behaviour and puts replaceValve() inside the same context as EquipmentSelectValves, so you get the outcome that you expect. Qt can't make
this the default yet because it will break old user code. But https://doc.qt.io/qt-6/qtqml-documents-structure.html#componentbehavior does say "The default value of ComponentBehavior is Unbound.... In a future version of Qt the default will change to Bound."

I haven't visited Monterey before, but I'll give you a buzz if I'm ever in town. That seafood sounds tempting ;-)