Skip to content

Remove AlchemyHTMLElement#3846

Merged
tvdeyen merged 8 commits intomainfrom
remove-alchemy-html-element
Apr 22, 2026
Merged

Remove AlchemyHTMLElement#3846
tvdeyen merged 8 commits intomainfrom
remove-alchemy-html-element

Conversation

@tvdeyen
Copy link
Copy Markdown
Member

@tvdeyen tvdeyen commented Apr 22, 2026

What is this pull request for?

The AlchemyHTMLElement was introduced to abstract some features for customElements into a shared base class. Mostly it was used to add reactivity to properties and attributes, but it turned out we never used is much and it just added overhead. Since it also queries the DOM in the constructor, something that's against the Custom Elements spec - we decided to remove it and inherit directly from HTMLElement instead.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@tvdeyen tvdeyen requested a review from a team as a code owner April 22, 2026 12:49
@tvdeyen tvdeyen added the breaking-change Breaking change label Apr 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.06%. Comparing base (dde947e) to head (830b02e).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3846   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         322      322           
  Lines        8530     8530           
=======================================
  Hits         8365     8365           
  Misses        165      165           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@sascha-karnatz sascha-karnatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far. I have only two questions:

Comment thread app/javascript/alchemy_admin/components/uploader/file_upload.js
Comment thread AGENTS.md
tvdeyen added 8 commits April 22, 2026 19:03
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement.
The base class reads innerHTML in its constructor, which violates the
Web Components spec and leaves cloned elements in an inconsistent
state when jQuery's clone(true) walks the subtree.

Spinner does not need any of the base class's abstractions: its size
and color attributes are set once at element creation (via the JS
Spinner wrapper or inline HTML) and never mutated afterwards, so
rendering only needs to run on connection.
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement.
The base class reads innerHTML in its constructor, which violates the
Web Components spec and leaves cloned elements in an inconsistent
state when jQuery's clone(true) walks the subtree.

Overlay only needs to render once on connection. Guard the text
interpolation with a nullish coalescing fallback so a missing text
attribute no longer renders the literal string "null".
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement.
The base class reads innerHTML in its constructor, which violates the
Web Components spec and leaves cloned elements in an inconsistent state
when jQuery's clone(true) walks the subtree.

Replace the maxChars static property with a simple getter that falls
back to 60 when the max-chars attribute is not set. The downstream
comparison with charLength still works because JavaScript's relational
operators coerce numeric strings to numbers.
Drop the AlchemyHTMLElement base class in favor of a plain HTMLElement.
The base class reads innerHTML in its constructor, which violates the
Web Components spec and leaves cloned elements in an inconsistent
state when jQuery's clone(true) walks the subtree.

Bail out of connectedCallback after the async locale import if the
element was disconnected in the meantime. Without this guard, a fast
drag-drop would leak a flatpickr calendar onto a detached input
because the drop ghost gets disconnected before the import resolves.
Also guard disconnectedCallback with optional chaining so a teardown
before initialization does not throw.
Dropping the AlchemyHTMLElement base class avoids the constructor-time
reading of innerHTML and attribute processing, which violated the Web
Components spec and contributed to the SortableJS drag-clone crash.

Attributes are now read through getters, Select2 is initialized in
connectedCallback with an isConnected guard after the async locale
import, and the instance is destroyed in disconnectedCallback.
Dropping the AlchemyHTMLElement base class avoids reading innerHTML at
construction time and rewriting it during render, which violated the
Web Components spec and contributed to the SortableJS drag-clone crash.

The spinner is now appended via insertAdjacentHTML with an idempotent
guard, so re-inserting the element does not stack multiple spinners.
All setup (classname, min-height, editor hiding, observer, theme
listener) is performed in connectedCallback.
Dropping the AlchemyHTMLElement base class avoids the constructor-time
innerHTML capture and attribute processing. The file input and dropzone
listeners, plus the self-registered Alchemy.upload.successful handler,
are now wired in connectedCallback.

The external dropzone element's listeners are removed in
disconnectedCallback, since they would otherwise hold closures that
keep the Uploader alive after removal. The dropzone attribute is
exposed via a plain getter.

FileUpload and Progress receive the same treatment. FileUpload now
sets its initial "in-progress" status through the status setter in
initialize(), so a status that is set before the element is appended
(e.g. status = "failed" in tests) is no longer clobbered when the
element connects. Progress pulls its template out into a module-level
function and uses private methods and fields for the internals that
are not part of the public contract.
No component extends it anymore after moving every custom element to
plain HTMLElement and connectedCallback. The base class encouraged the
very constructor-time DOM work that violated the Web Components spec
and broke cloneNode (SortableJS drag and drop). Dropping it removes
the tempt to repeat that pattern and the documentation now points new
components at extending HTMLElement directly.
@tvdeyen tvdeyen force-pushed the remove-alchemy-html-element branch from cbcbe05 to 830b02e Compare April 22, 2026 17:04
@tvdeyen tvdeyen enabled auto-merge April 22, 2026 17:06
@tvdeyen tvdeyen merged commit ebe1a66 into main Apr 22, 2026
27 of 28 checks passed
@tvdeyen tvdeyen deleted the remove-alchemy-html-element branch April 22, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants