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

fix: events handling #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cyfung1031
Copy link
Contributor

getEventTarget(xhr)

  • only document.createElement('a') was used.
  • document.createDocumentFragment() would be a better way to create EventTarget since the creation of HTMLElement is slightly higher than DocumentFragment
  • document.createDocumentFragment() is well supported in ES5. (just Opera 10-12 is unknown)
  • No functional affect to the current script.

addEventListener and removeEventListener

  • removeEventListener was missing, but mentioned in index.d.ts
  • WeakMap is used for this particular usage.
  • WeakMap in very old browsers might be not supported, in that case, removeEventListener is ignored.
  • No functional affect to the current script.

Minor fix in event handler

  • changed event.type = args[0]; to event.type = e.type; to avoid memory leakage of array args. the type must be the same as the args[0]. assignment not required.

cyfung1031 added a commit to cyfung1031/ajax-hook that referenced this pull request Aug 16, 2023
cyfung1031 added a commit to cyfung1031/ajax-hook that referenced this pull request Aug 16, 2023
cyfung1031 added a commit to cyfung1031/ajax-hook that referenced this pull request Aug 16, 2023
cyfung1031 added a commit to cyfung1031/ajax-hook that referenced this pull request Aug 16, 2023
cyfung1031 added a commit to cyfung1031/ajax-hook that referenced this pull request Aug 16, 2023
@cyfung1031
Copy link
Contributor Author

Combined Distribution Script ajaxhook.js for Testing (PR 119, 120, 121, 122)
https://cdn.jsdelivr.net/gh/cyfung1031/ajax-hook@1ebe48e08108449669290a226e52fc6fbf7ec9ef/dist/ajaxhook.js

@DAHUIAAAAA
Copy link
Collaborator

这次改动是要解决丢失 removeListener 的问题?

@cyfung1031
Copy link
Contributor Author

cyfung1031 commented Sep 2, 2023

这次改动是要解决丢失 removeListener 的问题?

之前只做了addEventListener,addEventListener跟removeEventListener是一對的
addEventListener裡面的把真正listen的function改了,一般removeEventListener是除掉不了

雖然很少機會會用removeEventListener,但你猜不到原本網站其他的XHR處理是怎樣,不理的話有機會報錯

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.

None yet

2 participants