ReactiveController: adding/removing controllers in lifecycle methods affects other controllers

[sorvell]

Which package(s) are affected?

Lit Core (lit / lit-html / lit-element / reactive-element)

Description

If a ReactiveController removes itself or adds another controller during a lifecycle method like hostUpdate or hostUpdated, this can prevent other controllers from properly running.

Reproduction

The initial text of the button in this repro should be the following and then clicking the button should not change the text.

Expected:

controller1.count: 1
controller2.count: 1

Actual:

controller1.count: 1
controller2.count: 0

(after clicking once)

controller1.count: 1
controller2.count: 1

Workaround

The issue is that the controllers are stored in an Array and to process the controllers' lifecycle, that array is forEach'd. Mutating the array while it's iterating unexpectedly alters the iteration.

The only workaround is not to alter the controllers list inside a controller lifecycle method.

This could possibly be addressed by (1) copying the controllers array before iterating, or (2) making the controllers a Set.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

All

Browser/OS/Node environment

Browser: All

[justinfagnani]

Ouch. Yeah, it's a concurrent modification bug. Set seems like the right way to go.