fix: "onload" event order

[benjamingr]

Deno has a bug where it currently always attaches .onload before addEventListener see this for a web example.

The following code:

let arr: any = [];
window.addEventListener('load', () => {
  arr.push(1);
});
window.onload = () => arr.push(2);
window.addEventListener('load', () => {
  arr.push(3);
});

window.addEventListener('load', () => {
  setTimeout(() => {
    console.log(arr);
  })
})

Logs [ 2, 1, 3 ] in deno whereas browsers (and deno with this patch) log [ 1, 2, 3 ]

Note that this can be a relatively risky change compared to the AbortSignal/WebSocket/Worker timing fixes

[bartlomieju]

LGTM, thanks @benjamingr

[benjamingr]

Thanks, sorry I'm a bit sick (and on my phone and not computer) - note that de-duplication here means that we return undefined and not null for things like onload which probably needs a separate fix.

[bartlomieju]

Thanks, sorry I'm a bit sick (and on my phone and not computer) - note that de-duplication here means that we return undefined and not null for things like onload which probably needs a separate fix.

@benjamingr thanks for pointing this out, should we hold on before merging then?

EDIT: I'm gonna revert my deduplication and leave a TODO comment to fix it in the future

[benjamingr]

@benjamingr thanks for pointing this out, should we hold on before merging then?

I think merging is probably fine it's an improvement I will follow up with another fix