refac: Multiple document.getElementById accross the codebase #405

Closed
opened 2025-11-11 14:20:26 -06:00 by GiteaMirror · 2 comments
Owner

Originally created by @Carlos-err406 on GitHub (Mar 3, 2024).

Bug Report

Description

Bug Summary:
there are many document.getElementById across the code see img.
this has performace and code cleanness issues as well as a safety impact, see additional information section below.

Steps to Reproduce:

Expected Behavior:
use a bind:this to the needed elements to save the element into a store so is more accessible, does less queries to the dom and keeps the code cleaner

Actual Behavior:
there are many queries to the DOM that could be avoided

Environment

  • Operating System: Windows
  • Browser (if applicable): not applicable

Reproduction Details

Confirmation:

  • I have read and followed all the instructions provided in the README.md.
  • I have reviewed the troubleshooting.md document.
  • I have included the browser console logs.
  • I have included the Docker container logs.

Logs and Screenshots

image

Browser Console Logs:
[Include relevant browser console logs, if applicable]

Docker Container Logs:
[Include relevant Docker container logs, if applicable]

Screenshots (if applicable):
[Attach any relevant screenshots to help illustrate the issue]

Installation Method

[Describe the method you used to install the project, e.g., manual installation, Docker, package manager, etc.]

Additional Information

Storing an element in a Svelte store offers several advantages over using document.getElementById throughout a codebase:

  • Performance: Storing the element in a store allows you to access it directly without needing to repeatedly query the DOM. This can improve performance by reducing the number of DOM queries and updates.
  • Safety: Storing the element in a store provides a centralized way to manage and access the element, reducing the risk of errors or inconsistencies that can occur when using document.getElementById in multiple places.
  • Code cleanliness: Storing the element in a store can make your codebase cleaner and more maintainable by encapsulating the logic for accessing and manipulating the element in one place.

Note

If the bug report is incomplete or does not follow the provided instructions, it may not be addressed. Please ensure that you have followed the steps outlined in the README.md and troubleshooting.md documents, and provide all necessary information for us to reproduce and address the issue. Thank you!

Originally created by @Carlos-err406 on GitHub (Mar 3, 2024). # Bug Report ## Description **Bug Summary:** there are many document.getElementById across the code see img. this has performace and code cleanness issues as well as a safety impact, see `additional information` section below. **Steps to Reproduce:** - **Expected Behavior:** use a bind:this to the needed elements to save the element into a store so is more accessible, does less queries to the dom and keeps the code cleaner **Actual Behavior:** there are many queries to the DOM that could be avoided ## Environment - **Operating System:** Windows - **Browser (if applicable):** not applicable ## Reproduction Details **Confirmation:** - [x] I have read and followed all the instructions provided in the README.md. - [x] I have reviewed the troubleshooting.md document. - [ ] I have included the browser console logs. - [ ] I have included the Docker container logs. ## Logs and Screenshots ![image](https://github.com/open-webui/open-webui/assets/81443707/913eeb15-9b54-4ab7-958c-2c69007497f5) **Browser Console Logs:** [Include relevant browser console logs, if applicable] **Docker Container Logs:** [Include relevant Docker container logs, if applicable] **Screenshots (if applicable):** [Attach any relevant screenshots to help illustrate the issue] ## Installation Method [Describe the method you used to install the project, e.g., manual installation, Docker, package manager, etc.] ## Additional Information Storing an element in a Svelte store offers several advantages over using document.getElementById throughout a codebase: - Performance: Storing the element in a store allows you to access it directly without needing to repeatedly query the DOM. This can improve performance by reducing the number of DOM queries and updates. - Safety: Storing the element in a store provides a centralized way to manage and access the element, reducing the risk of errors or inconsistencies that can occur when using document.getElementById in multiple places. - Code cleanliness: Storing the element in a store can make your codebase cleaner and more maintainable by encapsulating the logic for accessing and manipulating the element in one place. ## Note If the bug report is incomplete or does not follow the provided instructions, it may not be addressed. Please ensure that you have followed the steps outlined in the README.md and troubleshooting.md documents, and provide all necessary information for us to reproduce and address the issue. Thank you!
Author
Owner

@Carlos-err406 commented on GitHub (Mar 3, 2024):

i could contribute on this if thats ok

@Carlos-err406 commented on GitHub (Mar 3, 2024): i could contribute on this if thats ok
Author
Owner

@tjbck commented on GitHub (Mar 3, 2024):

I deliberately choose to use document.getElementById to allow more flexibility and some parts of the code that's the only way to have everything function correctly, but I see how the rest of the code could be replaced with bind:this without causing any issues, you just have to discern and find which ones. I believe this is a marginal improvement to the code and might introduce unintended bugs if not done properly so I'm hesitant to change, but the obvious part I believe we can make the switch, Feel free to make a PR!

@tjbck commented on GitHub (Mar 3, 2024): I deliberately choose to use `document.getElementById` to allow more flexibility and some parts of the code that's the only way to have everything function correctly, but I see how the rest of the code *could* be replaced with `bind:this` without causing any issues, you just have to discern and find which ones. I believe this is a marginal improvement to the code and might introduce unintended bugs if not done properly so I'm hesitant to change, but the obvious part I believe we can make the switch, Feel free to make a PR!
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/open-webui#405