JS: Should you use once to ensure mermaid doesn't init many times?

Created on 6 April 2023, over 1 year ago
Updated 8 September 2023, about 1 year ago

Problem/Motivation

I started a code review today and one of the first things I found was in js/diagram.js I noticed you're not using once. Once would ensure that a javascript you provide as a behavior only runs once. Without it, it would fire any time a behavior is used.

This might a problem you're avoiding in your limited use. But you could be introducing a problem for other since the javascript also isn't being applied to a specific context.

Based on my limited understanding of Drupal's behavior system, I think what this means is that your behavior is firing anytime anyone uses a behavior.

That doesn't appear to be your intent.

Steps to reproduce

1. install your module
2. put a breakpoint in your js method
3. Browse around your site and see if you can reach your breakpoint when you didn't expect to.

Proposed resolution

1. Add both the use of once and associate the init with a specific CSS selector that your module would insert into a page. That way your behavior would only fire when that selector is present. And only fire once.

Remaining tasks

Improve JS.

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Fixed

Version

1.0

Component

Diagram

Created by

πŸ‡ΊπŸ‡ΈUnited States cosmicdreams Minneapolis/St. Paul

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @cosmicdreams
  • πŸ‡ΊπŸ‡ΈUnited States swirt Florida

    Good catch. Thank you for reporting this.

  • πŸ‡ΊπŸ‡ΈUnited States swirt Florida
  • First commit to issue fork.
  • πŸ‡¦πŸ‡·Argentina tguerineau

    In order to address the issue I made a commit.

    Here's a brief overview of the changes:

    1. Refactored Mermaid JS Integration: Instead of initializing Mermaid for each diagram element multiple times, the code has been optimized to ensure each .mermaid element is initialized only once. This change not only boosts performance but also prevents potential rendering anomalies.

    2. Introduced jQuery's .once() Method: To achieve the above, I've incorporated the jQuery .once() method. This ensures that each .mermaid element is processed just a single time, avoiding redundant operations.

    By implementing these changes, the Mermaid JS file now runs efficiently, ensuring diagrams are processed correctly.

    Review the commit and provide feedback. If there are any concerns or further improvements to discuss, I'm open to suggestions.

  • Status changed to Needs review about 1 year ago
  • @tguerineau opened merge request.
  • Status changed to Fixed about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States swirt Florida

    Thank you for your contribution @tguerineau
    I tested this out and all diagrams render appropriately. This will go out with release 1.0.11

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024