fix(toast): initialize toaster state in initialize() so pre-existing targets don't throw

[pierry01]

Result

Problem

toaster_controller.js throws on page load whenever a toast target is already in the DOM at boot (e.g. a server-rendered flash message):

TypeError: Cannot read properties of undefined (reading 'set')
    at toastTargetConnected (application.js)
    at invokeControllerMethod
    at targetConnected

Stimulus fires target lifecycle callbacks in this order for pre-existing targets:

initialize() → targetConnected() → connect()

The controller initializes _heights / _resizeObservers inside connect(), but toastTargetConnected() calls this._heights.set(...) and this._resizeObservers.set(...). For a toast present in the initial HTML, toastTargetConnected runs before connect, so this._heights is still undefined → the whole controller (and often the whole Stimulus application) blows up.

This only manifests when a <Toast> is rendered server-side inside the <Toaster> on first paint. Toasts created at runtime via the stream action / JS API connect after connect() has run, which is why it slips through interactive testing.

Fix

Move the state that toastTargetConnected touches into initialize(), which is guaranteed to run before any targetConnected:

• _heights, _resizeObservers, _expanded are initialized in initialize().
• connect() still sets _expanded = this.expandValue (values are read in connect).
• Added a single this._reflow() at the end of connect() so server-rendered toasts get positioned: their toastTargetConnected ran before _listEl existed and _reflow() returned early via its if (!this._listEl) return guard.

No behavior change for runtime-created toasts.

Files

Both copies kept in sync:

• gem/lib/ruby_ui/toast/toaster_controller.js
• docs/app/javascript/controllers/ruby_ui/toaster_controller.js

How to reproduce

Render a Toaster with a Toast already inside it on initial page load (a flash message), then load the page → TypeError in the console before the fix, clean after.

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/app/javascript/controllers/ruby_ui/toaster_controller.js">

<violation number="1" location="docs/app/javascript/controllers/ruby_ui/toaster_controller.js:43">
P3: The docs and gem copies of toaster_controller.js, which the PR states are kept in sync, have drifted: this file reorders the property assignments in initialize() and drops the explanatory comment, while the gem copy still has the old order plus the comment. Behavior is identical, so unless the drift is intentional, mirror one to the other to keep the sync claim accurate and the intent comment preserved.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The docs and gem copies of toaster_controller.js, which the PR states are kept in sync, have drifted: this file reorders the property assignments in initialize() and drops the explanatory comment, while the gem copy still has the old order plus the comment. Behavior is identical, so unless the drift is intentional, mirror one to the other to keep the sync claim accurate and the intent comment preserved.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At docs/app/javascript/controllers/ruby_ui/toaster_controller.js, line 43:

<comment>The docs and gem copies of toaster_controller.js, which the PR states are kept in sync, have drifted: this file reorders the property assignments in initialize() and drops the explanatory comment, while the gem copy still has the old order plus the comment. Behavior is identical, so unless the drift is intentional, mirror one to the other to keep the sync claim accurate and the intent comment preserved.</comment>

<file context>
@@ -40,13 +40,9 @@ export default class extends Controller {
-    // can run safely for toasts already present in the DOM (e.g. server-rendered
-    // flash) before `connect` runs. Stimulus fires `targetConnected` before
-    // `connect` for pre-existing targets, so any state they touch must exist first.
+    this._expanded = false
     this._heights = new Map()
     this._resizeObservers = new WeakMap()
</file context>

[pierry01]

Resolved — the docs and gem copies are back in sync. The drift you caught was transient: it existed only between the two "Update toaster_controller.js" commits (the first touched only the docs copy). As of e4dc783 both files are byte-for-byte identical (diff -q confirms), with the same initialize() order and no comment. Behavior is unchanged. The MCP registry has also been rebuilt to match the current source.

[djalmaaraujo]

Reviewed this and the fix is correct — verified the ordering against the Stimulus 3.2.2 source rather than trusting the docs: Context calls controller.initialize() from its constructor, and Context#connect() starts the targetObserver before controller.connect(), so for a target already in the markup the order really is initialize() → toastTargetConnected() → connect(). Moving _heights / _resizeObservers into initialize() is the right layer to fix it at.

Worth noting the blast radius is bigger than the description suggests: ToastRegion(flash:) renders ToastItems straight into the <ol>, so the documented flash integration was a guaranteed way to hit this, and the TypeError takes down the whole Stimulus application, not just the toaster.

I pushed three follow-up commits to this branch — each one is self-contained, revert any you'd rather handle separately.

9ec2b6f — clear _listEl on disconnect. connect() resolved _listEl and disconnect() left it pointing at the detached <ol>. Stimulus tears down as controller.disconnect() → targetObserver.stop() → disconnectAllTargets(), so every toastTargetDisconnected (and the _reflow() it triggers) runs after the controller disconnected, writing inline styles onto nodes on their way out. Nulling _listEl lets _reflow()'s existing guard do its job.

This one needed four extra guards to avoid trading one crash for another: window.RubyUI.toast is a closure that outlives the controller and calls _dismissById / _mutate directly, not through the window listeners that disconnect() removes. With a null _listEl those .querySelector calls would throw where they previously returned null against the detached list — hence the optional chaining there and in _onKey, plus an early return in _spawn.

4dc2769 — positionValueChanged / expandValueChanged. Both are Stimulus values with nothing reacting to them: _spawn mirrored positionValue into data-position by hand, and expand was read once in connect(), so flipping either attribute at runtime did nothing until the next pointer event. The position callback now owns the data-position sync — that attribute is what region_classes keys its placement rules off, so reflowing without it would move the transforms and leave the container in the old corner. Both callbacks fire during valueObserver.start(), before connect(), and rely on the same if (!this._listEl) return guard the target callbacks do.

I deliberately kept _expanded = this.expandValue in connect(). It looks redundant next to expandValueChanged but isn't: StringMapObserver#stop doesn't clear its map and refreshAttribute only invokes the changed callback when the value differs from the cached one, so an unchanged attribute fires nothing when the observer restarts. The region is data-turbo-permanent, so it genuinely disconnects and reconnects with the same controller instance — without that line a toaster left hover-expanded would come back expanded.

e04c252 — idempotent toastTargetConnected. Now that _resizeObservers survives disconnect/reconnect cycles, a second toastTargetConnected for an element that still has an observer would overwrite the WeakMap entry and leak the first one, which keeps observing and keeps calling _reflow(). TargetObserver#connectTarget prevents that today by skipping elements already in targetsByName, but the callback shouldn't lean on that internal.

One accepted side effect: _spawn with detail.position now triggers an extra _reflow() via the value callback before the node is appended. It's idempotent, and repositioning the existing stack when the position changes is the correct behavior.

Gem suite is green (281 runs, 0 failures), standardrb clean, docs/app/javascript/controllers/ruby_ui/toaster_controller.js kept byte-identical, and mcp/data/registry.json rebuilt on each commit.