Skip to content

[WIP] Fix ESM script registration by name and subclass name collision#8832

Closed
Copilot wants to merge 1 commit into
mv-esm-script-registry-namefrom
copilot/fix-esm-script-registration
Closed

[WIP] Fix ESM script registration by name and subclass name collision#8832
Copilot wants to merge 1 commit into
mv-esm-script-registry-namefrom
copilot/fix-esm-script-registration

Conversation

Copy link
Copy Markdown

Copilot AI commented Jun 3, 2026

Thanks for the feedback on #8831. I've created this new PR, which merges into #8831, to address your comment. I will work on the changes and keep this PR's description up to date as I make progress.

Original PR: #8831
Triggering comment (#8831 (comment)):

Thanks for the review @copilot. Addressed the name-resolution edge cases (anonymous class with an empty .name and no static scriptName) in 05318dc:

Addressed (threads resolved):

  • toLowerCamelCase now returns the input unchanged for empty/falsy values instead of throwing on str[0]. This also defuses the two warning-string crashes (component.js and handlers/script.js).
  • registerScript logs an error and returns early when no name resolves, so it no longer assigns __name = undefined, calls add(), or pushes a nameless entry to ScriptTypes.
  • ScriptComponent.create returns null (and logs) instead of indexing the instance under an undefined/empty key (this.undefined).
  • ESM script handler skips exports whose name cannot be resolved rather than registering under an invalid key.

Added tests for the registerScript and ScriptComponent.create fail-fast paths.

Not addressed (left open):

  • The this._scripts.hasOwnProperty(...) suggestion in script-registry.js. It's a valid hardening point, but that line is pre-existing (untouched by this PR) and unrelated to the name-resolution fix, so it's out of scope here. Happy to do it as a separate small PR.

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.

2 participants