preline icon indicating copy to clipboard operation
preline copied to clipboard

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

Open oliverhaas opened this issue 1 year ago • 3 comments

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 avatar Aug 05 '24 21:08 oliverhaas

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 avatar Aug 05 '24 21:08 oliverhaas

Here a PR which addresses this issue.

https://github.com/htmlstreamofficial/preline/pull/441

oliverhaas avatar Aug 13 '24 20:08 oliverhaas

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

olegpix avatar Aug 28 '24 16:08 olegpix

Hey @oliverhaas - the destroy option is now available with the latest v2.6.0 release. Thank you for all your inputs!

jahaganiev avatar Dec 04 '24 00:12 jahaganiev