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

Use AbortController for input event cleanup #247

Merged
merged 4 commits into from
Nov 28, 2024

Conversation

le0pard
Copy link
Contributor

@le0pard le0pard commented Nov 17, 2024

Hello. Thanks for cool library.

I find out sometimes cleanup is not fully happen and dead events still exists on page after mask was destroyed. Possible reason of this issue, if on page exists DOM mutation lib, like https://github.com/bigskysoftware/idiomorph , which may use replaceChild to update elements (which may lead reference in map will not have old event).

Screenshot 2024-11-17 at 03 01 01

I decided will be faster and not loose any listener to use https://developer.mozilla.org/en-US/docs/Web/API/AbortController and just call it instead iterate over all created addEventListener (alpinejs example doing same - https://github.com/alpinejs/alpine/blob/main/packages/mask/src/index.js#L46 ).

Problem, that happy-dom not implemented AbortController for addEventListener, but it has jsdom. So I moved test, which check this functionality in separate file, where activate jsdom instead happy-dom (I tried activate jsdom for whole test suite, but one alpinejs tests is failing)

up: decided better figure out why alpine not working

up up: alpinejs test fixed

@le0pard
Copy link
Contributor Author

le0pard commented Nov 17, 2024

I did run tests with browser mode, and alpinejs "bindings > bind completed" still failing. so looks like issues with directive, not environment

@le0pard
Copy link
Contributor Author

le0pard commented Nov 19, 2024

Find out x-show not work correctly, so I replaced it by x-text. Now all tests should be green

@beholdr
Copy link
Owner

beholdr commented Nov 20, 2024

Hi, Oleksii

Thank you for your PR, I will examine it as soon as possible.

I like to use AbortController too. It's kind of funny that the line you a referring to is from my PR to alpinejs :) I tried to use it in maska, but found issue with happy-dom and didn't know that jsdom supports it. Thanks for this info.

@beholdr beholdr merged commit e3a601e into beholdr:master Nov 28, 2024
@beholdr
Copy link
Owner

beholdr commented Nov 28, 2024

Thank you again for contribution!

@le0pard le0pard deleted the improve-cleanup branch November 28, 2024 12:36
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

Successfully merging this pull request may close these issues.

2 participants