Skip to content

Commit

Permalink
fix: "onload" event order (denoland#8376)
Browse files Browse the repository at this point in the history
This commit fixes order of events for "onload" event. 

Previously handler attached using "window.onload" was
always fired before handlers added using "addEventListener".
  • Loading branch information
benjamingr authored Nov 26, 2020
1 parent e847049 commit 4f46dc9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 17 deletions.
53 changes: 40 additions & 13 deletions cli/rt/99_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,20 +283,47 @@ delete Object.prototype.__proto__;
Object.defineProperties(globalThis, mainRuntimeGlobalProperties);
Object.setPrototypeOf(globalThis, Window.prototype);
eventTarget.setEventTargetData(globalThis);
// Registers the handler for window.onload function.
globalThis.addEventListener("load", (e) => {
const { onload } = globalThis;
if (typeof onload === "function") {
onload(e);
}
});
// Registers the handler for window.onunload function.
globalThis.addEventListener("unload", (e) => {
const { onunload } = globalThis;
if (typeof onunload === "function") {
onunload(e);

const handlerSymbol = Symbol("eventHandlers");

function makeWrappedHandler(handler) {
function wrappedHandler(...args) {
if (typeof wrappedHandler.handler !== "function") {
return;
}
return wrappedHandler.handler.call(this, ...args);
}
});
wrappedHandler.handler = handler;
return wrappedHandler;
}
// TODO(benjamingr) reuse when we can reuse code between web crates
// This function is very similar to `defineEventHandler` in `01_web_util.js`
// but it returns `null` instead of `undefined` is handler is not defined.
function defineEventHandler(emitter, name) {
// HTML specification section 8.1.5.1
Object.defineProperty(emitter, `on${name}`, {
get() {
return this[handlerSymbol]?.get(name)?.handler ?? null;
},
set(value) {
if (!this[handlerSymbol]) {
this[handlerSymbol] = new Map();
}
let handlerWrapper = this[handlerSymbol]?.get(name);
if (handlerWrapper) {
handlerWrapper.handler = value;
} else {
handlerWrapper = makeWrappedHandler(value);
this.addEventListener(name, handlerWrapper);
}
this[handlerSymbol].set(name, handlerWrapper);
},
configurable: true,
enumerable: true,
});
}
defineEventHandler(window, "load");
defineEventHandler(window, "unload");

const { args, noColor, pid, ppid, unstableFlag } = runtimeStart();

Expand Down
4 changes: 2 additions & 2 deletions cli/tests/034_onload.out
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
log from nest_imported script
log from imported script
log from main
got load event in onload function
got load event in event handler (nest_imported)
got load event in event handler (imported)
got load event in event handler (main)
got unload event in onunload function
got load event in onload function
got unload event in event handler (nest_imported)
got unload event in event handler (imported)
got unload event in event handler (main)
got unload event in onunload function
4 changes: 2 additions & 2 deletions docs/runtime/program_lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ major difference between them, let's run the example:
$ deno run main.ts
log from imported script
log from main script
got load event in onload function (main)
got load event in event handler (imported)
got load event in event handler (main)
got unload event in onunload function (main)
got load event in onload function (main)
got unload event in event handler (imported)
got unload event in event handler (main)
got unload event in onunload function (main)
```

All listeners added using `window.addEventListener` were run, but
Expand Down

0 comments on commit 4f46dc9

Please sign in to comment.