Skip to content

HOLD: Gate remote_select style injection on an id to prevent <style> leak#1858

Draft
maebeale wants to merge 1 commit into
mainfrom
maebeale/fix-remote-select-style-leak
Draft

HOLD: Gate remote_select style injection on an id to prevent <style> leak#1858
maebeale wants to merge 1 commit into
mainfrom
maebeale/fix-remote-select-style-leak

Conversation

@maebeale

Copy link
Copy Markdown
Collaborator

What is the goal of this PR and why is this important?

  • remote_select_controller.js leaked a <style> tag on every Stimulus connect()
  • connect() ran document.head.appendChild(style) unconditionally — no "append once" guard and no disconnect() cleanup
  • Every connect (Turbo nav, cocoon add-field, reconnect) stacked another identical <style> into <head>, violating the lifecycle rule that anything created in connect() must be cleaned up

How did you approach the change?

  • Extracted the injection into an injectStyles() method gated on an element id (remote-select-styles)
  • If a tag with that id already exists, the method returns early — the shared global stylesheet is appended once and reused across all controller instances
  • Chose the id guard over a disconnect() removal because the stylesheet is global, not per-instance: multiple remote_select controllers can be live at once, so removing the tag on any one disconnect would break the others. The existing disconnect() still destroys the per-instance TomSelect.

Anything else to add?

  • HOLD: pre-existing issue surfaced while editing this block; holding for review/sequencing.

The connect() lifecycle appended an identical <style> tag on every connect
(Turbo nav, cocoon add, reconnect), leaking duplicates into document.head.
Gate the injection on an element id so the shared global stylesheet is
appended once and reused, satisfying Stimulus lifecycle discipline.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant