Skip to content

Conversation

@tvatavuk
Copy link
Contributor

fixes #5791

🧩 Summary

This PR fixes an intermittent JavaScript runtime error in Oqtane.Server/wwwroot/js/interop.js, where the variable script could remain undefined before attempting .remove().

In some custom module scenarios (especially when including or reloading inline scripts dynamically), includeScript() could execute along a code path that skipped initialization of the script variable, leading to:

TypeError: Cannot read properties of undefined (reading 'remove')

✅ Changes Made

  1. Initialize script to null to ensure safe declaration:

    var script = null;
  2. Add defensive check before calling .remove():

    if (script instanceof HTMLScriptElement) {
        script.remove();
    }

Copilot AI review requested due to automatic review settings November 10, 2025 13:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies the includeScript function in the JavaScript interop file to improve type safety. The changes initialize the script variable explicitly to null and replace a null check with an instanceof type check when determining whether to remove an existing script element.

  • Explicit initialization of the script variable to null
  • Type-checking using instanceof HTMLScriptElement instead of null comparison
Comments suppressed due to low confidence (1)

Oqtane.Server/wwwroot/js/interop.js:147

  • Inconsistent condition logic creates a potential bug. Line 143 uses instanceof HTMLScriptElement while line 147 checks script === null. If querySelector or getElementById returns a truthy value that is not an HTMLScriptElement (edge case), line 143 would not execute (script wouldn't be removed or set to null), but line 147 would also not execute (won't create new script). This breaks the function's logic. Both conditions should use the same check - either both use !== null/=== null or both use instanceof HTMLScriptElement/!(script instanceof HTMLScriptElement).
        if (script instanceof HTMLScriptElement) {
            script.remove();
            script = null;
        }
        if (script === null) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sbwalker sbwalker merged commit 62db107 into oqtane:dev Nov 10, 2025
7 checks passed
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.

[BUG] TypeError: Cannot read properties of undefined (reading 'remove')

2 participants