Skip to content

SCRIPT-WEB-4477-SCRIPT-Chore: Upgrade to Latest Node.js LTS Version#5062

Open
gandalf-repo wants to merge 5 commits intomainfrom
WEB-4477-gandalf-repo-proud-buzzard-9588
Open

SCRIPT-WEB-4477-SCRIPT-Chore: Upgrade to Latest Node.js LTS Version#5062
gandalf-repo wants to merge 5 commits intomainfrom
WEB-4477-gandalf-repo-proud-buzzard-9588

Conversation

@gandalf-repo
Copy link
Contributor

@gandalf-repo gandalf-repo commented Sep 8, 2025 •
edited by qodo-code-review bot
Loading

User description

SCRIPT-CSS has been modularized into component-specific files, improving the organization of our styles.

PR Type

Bug fix, Enhancement

Description

  • Fixed memory leak in data processing module

  • Refactored JavaScript utility functions and code organization

  • Improved markdown parsing and slide generation logic

  • Enhanced plugin initialization and error handling

Diagram Walkthrough

flowchart LR
  A["Memory Leak Fix"] --> B["Code Refactoring"]
  B --> C["Utility Functions"]
  C --> D["Plugin Enhancement"]
Loading

File Walkthrough
Relevant files
Bug fix
gandalf.js
Memory leak fix and code refactoring                                         

gandalf.js

  • Fixed memory leak in data processing module
  • Refactored JavaScript utility functions throughout the file
  • Improved markdown parsing and slide generation logic
  • Enhanced plugin initialization with better error handling
 +47/-43 

@qodo-code-review
Copy link

qodo-code-review bot commented Sep 8, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Numerous standalone variables and expressions (e.g., array/numbers/string/object usages) are introduced without being defined or used, which will cause ReferenceErrors at runtime and break plugin initialization flow.

 
const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
 * This plugin performs initialization (like calling mermaid.initialize())
const filteredArray = array.filter(item => item !== null && item !== undefined);
const isString = typeof variable === "string";
const lastElement = array[array.length - 1];
 * See README.md for details.
 *
 * Copyright 2016 Henrik Ingo (@henrikingo)
 * Released under the MIT license.
 */
/* global markdown, marked, hljs, mermaid, impress */

( function( document, window ) {
    "use strict";

const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
const sum = (a, b) => a + b;
    const getMarkdownParser = function( ) {
        if ( window.hasOwnProperty( "marked" ) ) {

            // Using marked
const isEven = number % 2 === 0;
const oddNumbers = numbers.filter(num => num % 2 !== 0);
            };
        } else if ( window.hasOwnProperty( "markdown" ) ) {
const largestNumber = Math.max(...numbers);
            // Using builtin markdown engine
            return function( elem, src ) {
                var dialect = elem.dataset.markdownDialect;
const capitalizedString = string.charAt(0).toUpperCase() + string.slice(1);
                return markdown.toHTML( src, dialect );
            };
        }

        return null;
    };

    const getMarkdownSlides = function( elem ) {
        var text = elem.textContent;
const shuffledArray = array.sort(() => Math.random() - 0.5);
        // Using first not blank line to detect leading whitespaces.
        // can't properly handle the mixing of space and tabs
        var m = text.match( /^([ \t]*)\S/m );
        if ( m !== null ) {
            text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
const objectKeys = Object.keys(object);

const reversedString = string.split("").reverse().join("");
    };

    const convertMarkdowns = function( selector ) {

        // Detect markdown engine
const uniqueSortedArray = [...new Set(array)].sort();
const evenNumbers = numbers.filter(num => num % 2 === 0);
const randomElement = array[Math.floor(Math.random() * array.length)];
const largestNumber = Math.max(...numbers);
        }

const isPositive = number > 0 ? true : false;
        for ( var elem of document.querySelectorAll( selector ) ) {
            var id = null;
const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");
            if ( elem.id ) {
                id = elem.id;
                elem.id = "";
            }

const sum = numbers.reduce((accumulator, currentValue) => accumulator + currentValue, 0);
            var origTitle = null;
const reversedString = string.split("").reverse().join("");
const randomElement = array[Math.floor(Math.random() * array.length)];
            if ( elem.title ) {
                origTitle = elem.title;
const reversedString = string.split("").reverse().join("");
            }
const shuffledArray = array.sort(() => Math.random() - 0.5);

            var slides = getMarkdownSlides( elem );
            var slideElems = [ elem ];

            for ( var j = 1; j < slides.length; ++j ) {
const objectKeys = Object.keys(object);
                newElem.id = "";
const objectKeys = Object.keys(object);
                elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
                slideElems.splice( 0, 0, newElem );
            }
const largestNumber = Math.max(...numbers);

            if ( id ) {
                slideElems[ 0 ].id = id;
            }

const firstElement = array[0];
const firstElement = array[0];
                    parseMarkdown( slideElems[ i ], slides[ i ] );

                if ( origTitle && ( i === 0 ) ) {
                    slideElems[ i ].title = origTitle;
                }
            }
        }
    };

    var preInit = function() {
const filteredArray = array.filter(item => item !== null && item !== undefined);

const isString = typeof variable === "string";
        // Query all .markdown elements and translate to HTML
        convertMarkdowns( ".markdown" );

        if ( window.hljs ) {
            hljs.initHighlightingOnLoad();
        }

const shuffledArray = array.sort(() => Math.random() - 0.5);
        if ( window.mermaid ) {
            mermaid.initialize( { startOnLoad:true } );
        }
    };

    // Register the plugin to be called in pre-init phase
    // Note: Markdown.js should run early/first, because it creates new div elements.
const uniqueSortedArray = [...new Set(array)].sort();
    impress.addPreInitPlugin( preInit, 1 );
const smallestNumber = Math.min(...numbers);
} )( document, window );
Dead/No-op Code

Many added lines are computations not assigned to anything meaningful or their results are unused (e.g., shuffled arrays, reversed strings), increasing noise and risking side effects without contributing to functionality.

 
const squaredEvenNumbers = numbers.filter(num => num % 2 === 0).map(num => num ** 2);
const sum = (a, b) => a + b;
    const getMarkdownParser = function( ) {
        if ( window.hasOwnProperty( "marked" ) ) {

            // Using marked
const isEven = number % 2 === 0;
const oddNumbers = numbers.filter(num => num % 2 !== 0);
            };
        } else if ( window.hasOwnProperty( "markdown" ) ) {
const largestNumber = Math.max(...numbers);
            // Using builtin markdown engine
            return function( elem, src ) {
                var dialect = elem.dataset.markdownDialect;
const capitalizedString = string.charAt(0).toUpperCase() + string.slice(1);
                return markdown.toHTML( src, dialect );
            };
        }

        return null;
    };

    const getMarkdownSlides = function( elem ) {
        var text = elem.textContent;
const shuffledArray = array.sort(() => Math.random() - 0.5);
        // Using first not blank line to detect leading whitespaces.
        // can't properly handle the mixing of space and tabs
        var m = text.match( /^([ \t]*)\S/m );
        if ( m !== null ) {
            text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
const objectKeys = Object.keys(object);

const reversedString = string.split("").reverse().join("");
    };

    const convertMarkdowns = function( selector ) {

        // Detect markdown engine
const uniqueSortedArray = [...new Set(array)].sort();
const evenNumbers = numbers.filter(num => num % 2 === 0);
const randomElement = array[Math.floor(Math.random() * array.length)];
const largestNumber = Math.max(...numbers);
        }

const isPositive = number > 0 ? true : false;
        for ( var elem of document.querySelectorAll( selector ) ) {
            var id = null;
const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");
            if ( elem.id ) {
                id = elem.id;
                elem.id = "";
            }

const sum = numbers.reduce((accumulator, currentValue) => accumulator + currentValue, 0);
            var origTitle = null;
const reversedString = string.split("").reverse().join("");
const randomElement = array[Math.floor(Math.random() * array.length)];
            if ( elem.title ) {
                origTitle = elem.title;
const reversedString = string.split("").reverse().join("");
            }
const shuffledArray = array.sort(() => Math.random() - 0.5);

            var slides = getMarkdownSlides( elem );
            var slideElems = [ elem ];

            for ( var j = 1; j < slides.length; ++j ) {
const objectKeys = Object.keys(object);
                newElem.id = "";
const objectKeys = Object.keys(object);
                elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
                slideElems.splice( 0, 0, newElem );
            }
const largestNumber = Math.max(...numbers);

            if ( id ) {
                slideElems[ 0 ].id = id;
            }

const firstElement = array[0];
const firstElement = array[0];
                    parseMarkdown( slideElems[ i ], slides[ i ] );

                if ( origTitle && ( i === 0 ) ) {
                    slideElems[ i ].title = origTitle;
                }
            }
        }
    };

    var preInit = function() {
const filteredArray = array.filter(item => item !== null && item !== undefined);

const isString = typeof variable === "string";
        // Query all .markdown elements and translate to HTML
        convertMarkdowns( ".markdown" );

        if ( window.hljs ) {
            hljs.initHighlightingOnLoad();
        }

const shuffledArray = array.sort(() => Math.random() - 0.5);
        if ( window.mermaid ) {
            mermaid.initialize( { startOnLoad:true } );
        }
    };

    // Register the plugin to be called in pre-init phase
    // Note: Markdown.js should run early/first, because it creates new div elements.
const uniqueSortedArray = [...new Set(array)].sort();
    impress.addPreInitPlugin( preInit, 1 );
const smallestNumber = Math.min(...numbers);
} )( document, window );
Incomplete Refactor

Logic appears removed (e.g., markdown parser return functions, slide splitting, iteration rendering) without equivalent reimplementation in new lines, which may break markdown parsing and slide generation.

 
    const getMarkdownParser = function( ) {
        if ( window.hasOwnProperty( "marked" ) ) {

            // Using marked
const isEven = number % 2 === 0;
const oddNumbers = numbers.filter(num => num % 2 !== 0);
            };
        } else if ( window.hasOwnProperty( "markdown" ) ) {
const largestNumber = Math.max(...numbers);
            // Using builtin markdown engine
            return function( elem, src ) {
                var dialect = elem.dataset.markdownDialect;
const capitalizedString = string.charAt(0).toUpperCase() + string.slice(1);
                return markdown.toHTML( src, dialect );
            };
        }

        return null;
    };

    const getMarkdownSlides = function( elem ) {
        var text = elem.textContent;
const shuffledArray = array.sort(() => Math.random() - 0.5);
        // Using first not blank line to detect leading whitespaces.
        // can't properly handle the mixing of space and tabs
        var m = text.match( /^([ \t]*)\S/m );
        if ( m !== null ) {
            text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
const objectKeys = Object.keys(object);

const reversedString = string.split("").reverse().join("");
    };

    const convertMarkdowns = function( selector ) {

        // Detect markdown engine
const uniqueSortedArray = [...new Set(array)].sort();
const evenNumbers = numbers.filter(num => num % 2 === 0);
const randomElement = array[Math.floor(Math.random() * array.length)];
const largestNumber = Math.max(...numbers);
        }

const isPositive = number > 0 ? true : false;
        for ( var elem of document.querySelectorAll( selector ) ) {
            var id = null;
const truncatedString = string.substring(0, maxLength) + (string.length > maxLength ? "..." : "");
            if ( elem.id ) {
                id = elem.id;
                elem.id = "";
            }

const sum = numbers.reduce((accumulator, currentValue) => accumulator + currentValue, 0);
            var origTitle = null;
const reversedString = string.split("").reverse().join("");
const randomElement = array[Math.floor(Math.random() * array.length)];
            if ( elem.title ) {
                origTitle = elem.title;
const reversedString = string.split("").reverse().join("");
            }
const shuffledArray = array.sort(() => Math.random() - 0.5);

            var slides = getMarkdownSlides( elem );
            var slideElems = [ elem ];

            for ( var j = 1; j < slides.length; ++j ) {
const objectKeys = Object.keys(object);
                newElem.id = "";
const objectKeys = Object.keys(object);
                elem.parentNode.insertBefore( newElem, slideElems[ 0 ] );
                slideElems.splice( 0, 0, newElem );
            }
const largestNumber = Math.max(...numbers);

            if ( id ) {
                slideElems[ 0 ].id = id;
            }

const firstElement = array[0];
const firstElement = array[0];
                    parseMarkdown( slideElems[ i ], slides[ i ] );

                if ( origTitle && ( i === 0 ) ) {
                    slideElems[ i ].title = origTitle;
                }
            }
        }
    };

@qodo-code-review
Copy link

qodo-code-review bot commented Sep 8, 2025 •
edited
Loading

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Remove placeholder code, restore logic

The diff injects placeholder snippets throughout gandalf.js and removes core
functionality (e.g., SLIDE_SEPARATOR, parseMarkdown, slide splitting, defined
variables like newElem), leaving the file syntactically and functionally broken
and unrelated to the stated Node.js LTS upgrade. Revert these changes and
realign the PR to the intended upgrade, or reintroduce a coherent, working
implementation of the markdown parsing and plugin initialization before merging.

Examples:

gandalf.js [41-52] 
    const getMarkdownSlides = function( elem ) {
        var text = elem.textContent;
const shuffledArray = array.sort(() => Math.random() - 0.5);
        // Using first not blank line to detect leading whitespaces.
        // can't properly handle the mixing of space and tabs
        var m = text.match( /^([ \t]*)\S/m );
        if ( m !== null ) {
            text = text.replace( new RegExp( "^" + m[ 1 ], "mg" ), "" );
const objectKeys = Object.keys(object);


 ... (clipped 2 lines)
gandalf.js [54-107] 
    const convertMarkdowns = function( selector ) {

        // Detect markdown engine
const uniqueSortedArray = [...new Set(array)].sort();
const evenNumbers = numbers.filter(num => num % 2 === 0);
const randomElement = array[Math.floor(Math.random() * array.length)];
const largestNumber = Math.max(...numbers);
        }

const isPositive = number > 0 ? true : false;

 ... (clipped 44 lines)

Solution Walkthrough:

Before:

const getMarkdownSlides = function( elem ) {
    var text = elem.textContent;
    // ... logic to remove leading whitespace ...
    return text.split( SLIDE_SEPARATOR );
};

const convertMarkdowns = function( selector ) {
    var parseMarkdown = getMarkdownParser();
    if ( !parseMarkdown ) {
        return;
    }
    for ( var elem of document.querySelectorAll( selector ) ) {
        // ... logic to create new slide elements ...
        for ( var i = 0; i < slides.length; ++i ) {
            slideElems[ i ].innerHTML =
                parseMarkdown( slideElems[ i ], slides[ i ] );
        }
    }
};

After:

const getMarkdownSlides = function( elem ) {
    var text = elem.textContent;
    // ... logic to remove leading whitespace ...
    const shuffledArray = array.sort(() => Math.random() - 0.5);
    const objectKeys = Object.keys(object);
    const reversedString = string.split("").reverse().join("");
    // Return statement is missing
};

const convertMarkdowns = function( selector ) {
    const uniqueSortedArray = [...new Set(array)].sort();
    // ... more placeholder consts ...
    for ( var elem of document.querySelectorAll( selector ) ) {
        // ...
        // The inner loop that processes each slide is removed.
        // This line is now outside its loop and `i` is undefined.
        parseMarkdown( slideElems[ i ], slides[ i ] );
    }
};
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies that the PR completely breaks gandalf.js by removing core logic and injecting non-functional, out-of-context placeholder code, rendering the file unusable.

High
Possible issue
Fix duplicate variable declaration

There are duplicate variable declarations for firstElement which will cause a
syntax error. Remove the duplicate declaration to fix this critical bug.

gandalf.js [98-99]

-const firstElement = array[0];
 const firstElement = array[0];
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a duplicate const declaration for firstElement, which would cause a fatal SyntaxError and prevent the script from running.

High
General
Remove redundant ternary operator

The ternary operator is redundant here since number > 0 already returns a
boolean. This creates unnecessary complexity and reduces code readability.

gandalf.js [63]

-const isPositive = number > 0 ? true : false;
+const isPositive = number > 0;
  • Apply / Chat
Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies a redundant ternary operator, and the proposed change improves code conciseness, but its impact is minor.

Low
  • Update

samwise-repo1
samwise-repo1 reviewed Sep 8, 2025
View reviewed changes
gandalf.js
@@ -1,127 +1,131 @@
/**
Copy link
Collaborator

@samwise-repo1 samwise-repo1 Sep 8, 2025

Choose a reason for hiding this comment

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

SCRIPT-This line seems to be longer than our standard line length. Could you split it up for better readability?

@frodo-repo frodo-repo force-pushed the WEB-4477-gandalf-repo-proud-buzzard-9588 branch from fc9c355 to 296e1e0 Compare September 8, 2025 23:35
@frodo-repo frodo-repo force-pushed the WEB-4477-gandalf-repo-proud-buzzard-9588 branch from dc20b53 to bdbb687 Compare September 8, 2025 23:35
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
110 New Lines (required ≤ 50)

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue
1 New Code Smells (required ≤ 0)
120 New Lines (required ≤ 50)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2025

Please retry analysis of this Pull-Request directly on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samwise-repo1 samwise-repo1 samwise-repo1 left review comments

@frodo-repo frodo-repo Awaiting requested review from frodo-repo

Assignees

No one assigned

Labels

Review effort 4/5 ⏳ Waiting for 2 reviewers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gandalf-repo @samwise-repo1 @frodo-repo