Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Advanced Select, Dropdown, ...] Memory leak when calling autoInit() repeatedly (e.g. due to ajax) #429

Open
oliverhaas opened this issue Aug 5, 2024 · 3 comments

Comments

@oliverhaas
Copy link

Summary

Calling autoInit() repeatedly will lead to a memory leak

Steps to Reproduce

  1. Have a page with Advanced Select or Dropdown (possibly more)
  2. Use autoInit() with ajax like described in the docs https://preline.co/docs/preline-javascript.html
  3. With every ajax request, duplicate event listeners are registered on the window
    image

Demo Link

None

Expected Behavior

No response

Actual Behavior

No response

Screenshots

No response

@oliverhaas
Copy link
Author

oliverhaas commented Aug 5, 2024

The code responsible is (Advanced Select as an example, comments by me)

        // src/plugins/select/index.ts
	static autoInit() {
		if (!window.$hsSelectCollection) window.$hsSelectCollection = [];

		document
			.querySelectorAll('[data-hs-select]:not(.--prevent-on-load-init)')
			.forEach((el: HTMLElement) => {
				if (
					!window.$hsSelectCollection.find(
						(elC) => (elC?.element?.el as HTMLElement) === el,
					)
				) {
					const data = el.getAttribute('data-hs-select');
					const options: ISelectOptions = data ? JSON.parse(data) : {};

					new HSSelect(el, options);
				}
			});

                // this code block is basically always run after the first autoInit(), and registers the same event listeners over and over again.
		if (window.$hsSelectCollection) {
			window.addEventListener('click', (evt) => {
				const evtTarget = evt.target;

				HSSelect.closeCurrentlyOpened(evtTarget as HTMLElement);
			});

			document.addEventListener('keydown', (evt) =>
				HSSelect.accessibility(evt),
			);
		}
	}

From what I can tell, a quick fix would look like this

	static autoInit() {
		if (!window.$hsSelectCollection) {
			window.$hsSelectCollection = [];
			window.addEventListener('click', (evt) => {
				const evtTarget = evt.target;

				HSSelect.closeCurrentlyOpened(evtTarget as HTMLElement);
			});

			document.addEventListener('keydown', (evt) =>
				HSSelect.accessibility(evt),
			);
		}

		document
			.querySelectorAll('[data-hs-select]:not(.--prevent-on-load-init)')
			.forEach((el: HTMLElement) => {
				if (
					!window.$hsSelectCollection.find(
						(elC) => (elC?.element?.el as HTMLElement) === el,
					)
				) {
					const data = el.getAttribute('data-hs-select');
					const options: ISelectOptions = data ? JSON.parse(data) : {};

					new HSSelect(el, options);
				}
			});
	}

Sorry, I can't provide more right now. I hope it's enough for now to get this out of the door.

@oliverhaas
Copy link
Author

oliverhaas commented Aug 13, 2024

Here a PR which addresses this issue.

#441

@olegpix
Copy link
Collaborator

olegpix commented Aug 28, 2024

@oliverhaas Hi,
Thank you for your work, we really appreciate it.
We will check the PR and approve it as soon as possible.

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

No branches or pull requests

2 participants