Fix: resolve CustomElementRegistry duplicate constructor error#568
Fix: resolve CustomElementRegistry duplicate constructor error#568raman325 merged 3 commits intoFutureTense:mainfrom
Conversation
CustomElementRegistry.define() requires a unique constructor per element name. The alias registrations reused the same class constructors as the primary registrations, causing a runtime error in browsers. Create thin subclasses for the aliases so each define() call gets a unique constructor. Also exclude generated JS from codespell checks since minified variable names trigger false positives. Co-Authored-By: GitHub Copilot <copilot@github.com> Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical runtime error in the Home Assistant frontend where the CustomElementRegistry was throwing "this constructor has already been used with this registry" errors. The fix creates thin subclasses for alias registrations since customElements.define() requires a unique constructor per element name. Additionally, the PR excludes the generated/minified JavaScript from codespell checks to prevent false positives from minified variable names.
Changes:
- Created unique subclasses (
KeymasterDashboardStrategyAlias,KeymasterViewStrategyAlias,KeymasterSectionStrategyAlias) for alias element registrations - Added guards using
customElements.get()to prevent duplicate registration errors when the script is loaded multiple times - Excluded
custom_components/keymaster/www/generated/from codespell checks in bothpyproject.tomland.pre-commit-config.yaml
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lovelace_strategy/main.ts |
Added thin subclasses for alias registrations and guards to prevent duplicate registrations |
custom_components/keymaster/www/generated/keymaster.js |
Regenerated minified JavaScript reflecting the TypeScript changes with new subclasses |
pyproject.toml |
Added skip configuration for codespell to exclude generated files |
.pre-commit-config.yaml |
Added exclude pattern for codespell pre-commit hook to skip generated files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add customElements.get() guards to the primary registrations for consistency with the alias registrations. This prevents errors if the script is loaded more than once. Co-Authored-By: GitHub Copilot <copilot@github.com> Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #568 +/- ##
==========================================
+ Coverage 84.14% 84.44% +0.29%
==========================================
Files 10 32 +22
Lines 801 2822 +2021
Branches 0 30 +30
==========================================
+ Hits 674 2383 +1709
- Misses 127 439 +312
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
raman325
left a comment
raman325
left a comment
There was a problem hiding this comment.
thanks for fixing this - does tha mean tha liases were broken?
|
Honestly, I'm not certain. I happened across the problem while working on #569 and also saw the issue after I was about to raise the PR so and then broke it apart as I thought I had introduced it at first! I do notice that Codecov is complaining about no coverage, so to appease @firstof9 I'll get some tests added to this fix ;) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add tests for main.ts verifying: - All six custom elements are registered on first load - Registration is skipped when elements are already defined - Alias constructors are distinct from primary constructors Co-Authored-By: GitHub Copilot <copilot@github.com> Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
lol when it comes to the frontend side of things, I have no idea how any of that would work, I only care about the python side myself 😝 |
You can still rightly complain that my first PR didn't even include tests! |
Summary
CustomElementRegistry.define() requires a unique constructor per element
name. The alias registrations reused the same class constructors as the
primary registrations, causing a runtime error in browsers. Create thin
subclasses for the aliases so each define() call gets a unique constructor.
Also exclude generated JS from codespell checks since minified variable
names trigger false positives.
Type of change
Additional information
keymaster.js#567