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

attachEvent is not need #474

Open
ocknamo opened this issue May 13, 2022 · 3 comments
Open

attachEvent is not need #474

ocknamo opened this issue May 13, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@ocknamo
Copy link

ocknamo commented May 13, 2022

Thanks a lot for happy-dom.

I think I have discovered that happy-dom is creating attachEvents.

// In setup-jest.ts

// @ts-ignore
if(attachEvent) {
  // @ts-ignore
  console.log(typeof attachEvent); // log >> function
}

I think attachEvent is unnecessary because it is an old IE only implementation.
Also, when the test is run, polyfills from other libraries are executed and an error occurs. (eg. socket.io-client)

It is not a high priority because it can be avoided as follows.

// @ts-ignore
if(typeof attachEvent === 'function') {
  // @ts-ignore
  attachEvent = undefined;
}

Thanks.

@capricorn86
Copy link
Owner

Thanks for reporting @ocknamo! 🙂

attachEvent() was added to solve some weird scenario in React together with testing-library. I don't remember exactly why it was needed, but I remember that it solved the problem. I am a bit worried what the consequence would be if I remove it 😅

It was added to solve the problems in this issue:
#313

Do you have some examples of libraries that fails?

@capricorn86 capricorn86 added the bug Something isn't working label May 16, 2022
@capricorn86 capricorn86 self-assigned this May 16, 2022
@fsoikin
Copy link
Contributor

fsoikin commented Aug 5, 2022

This is related to the lack of support for the input event.

  • Happy DOM doesn't support the input event.
  • React, in turn, uses the absence of the input event to infer that it's running in IE 9.
  • And if it's running in IE 9, it assumes support for the attachEvent and detachEvent methods, which Happy DOM also doesn't support.

Well, now apparently it supports attachEvent, which I'm guessing solved somebody's particular scenario that didn't involve detachEvent.

But the root problem is the lack of support for input. And here's the other half of the symptoms: #534

@fsoikin
Copy link
Contributor

fsoikin commented Aug 5, 2022

Oh, actually, that's not exactly correct.
It appears that it's not so much about support for input event per se as how React determines such support.

What React does is this (source):

    var element = document.createElement('div');
    element.setAttribute(eventName, 'return;');
    isSupported = typeof element[eventName] === 'function';

It attempts to assign an event handler by setting an attribute and then checks if it got turned into a function, which is what a browser would do:

> d = document.createElement("div")
  <div></div>
> d.setAttribute("onclick", "return;")
undefined
> d.onclick
ƒ onclick(event) {
return;
}

But happy-dom doesn't do that:

 λ node
Welcome to Node.js v16.15.0.
Type ".help" for more information.
> dom = require("@happy-dom/global-registrator")
{ GlobalRegistrator: [class GlobalRegistrator] { registered: [] } }
> dom.GlobalRegistrator.register()
undefined
> d = document.createElement("div")
...
> d.setAttribute("onclick", "return;")
undefined
> d.onclick
undefined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants