C7 properties panel breaks when template variable assignment is disabled

[barmac]

Describe the bug

C7 properties panel freezes when template variable assignment is disabled. The error thrown is related to the inputOutput missing unexpectedly, or outputParameter missing unexpectedly if there are more outputs on the element.

Steps to reproduce

1. Apply the template below:

[
  {
    "name": "output-test",
    "id": "output-test",
    "appliesTo": [
      "bpmn:Task"
    ],
    "properties": [
      {
        "label": "output",
        "description": "disable assignment to reproduce",
        "binding": {
          "type": "camunda:outputParameter",
          "source": "name"
        }
      }
    ]
  }
]

2. Switch toggle for variable assignment
3. Properties panel freezes, and it does not update with element selection

Expected behavior

No crash.

Environment

• OS: [e.g. MacOS 10.2, Windows 10]
• Camunda Modeler Version: 5.24.0 but not 5.23.0
• Execution Platform: C7
• Installed plug-ins: [...]

Additional context

This is a regression.

SUPPORT-22412

[barmac]

This is first broken in bpmn-io/bpmn-js-element-templates@86a3c03

[barmac]

I suspect bpmn-io/properties-panel@bb1cfb0 as there were no big changes in the templates project in the marked commit apart from @bpmn-io/properties-panel version bump.

[barmac]

The direct cause of the problem is an unhandled error in the rendering when we try to get camunda:name of a non-existing output parameter. Somehow after the parameter is removed, we still try to render this component, although it should not be rendered according to this if: https://github.com/bpmn-io/bpmn-js-element-templates/blob/main/src/element-templates/properties-panel/properties/OutputProperties.js#L67
So this is a race condition. Correct flow is: toggle switch -> remove parameter -> render without AssignToProcessVariable; the actual flow is: toggle switch -> remove parameter -> rerender AssignToProcessVariable -> 💣

[barmac]

I've just confirmed that the bug appears when #4382 (comment) is included.

[barmac]

My current hypothesis is that the incorrect rerender is due to naive key implementation which is based on the index of the object.

[barmac]

My current hypothesis is that the incorrect rerender is due to naive key implementation which is based on the index of the object.

Setting random keys doesn't help.

[barmac]

Today's findings from a session with @nikku :

ListGroup.js

  // we need to pass the proper (initial) open state when we 
  // initially render an element
  // 
  // at the same time, we compute the state _only_ in the second PASS
  // 
  // ==> So essentially, by design, we render _once_ with the old state.
  // ==> BOOM (!)

[nikku]

I have a bug fix in place that removes the aweful hacks we add in ListGroup:

To be integration tested (manually) tomorrow.

[nikku]

Turned out, as reported, to be a ListGroup related issue (old components used for re-rendering against new model).

Fixed upstream via bpmn-io/properties-panel#369.

[marstamm]

Closed through #4392