htmlstreamofficial / preline

Preline UI is an open-source set of prebuilt UI components based on the utility-first Tailwind CSS framework.
https://preline.co
Other
4.9k stars 309 forks source link

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

Open oliverhaas opened 3 months ago

oliverhaas commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

Here a PR which addresses this issue.

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

olegpix commented 2 months ago

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