closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Unknown type 'BeforeUnloadEvent'

Open supahero1 opened this issue 3 years ago • 3 comments

test.js:

/** @type {BeforeUnloadEvent} */
var a;

Compiled with closure-compiler -O ADVANCED --js test.js --js_output_file test.min.js version v20220601:

test.js:1:11: WARNING - [JSC_UNRECOGNIZED_TYPE_ERROR] Bad type annotation. Unknown type BeforeUnloadEvent
  1| /** @type {BeforeUnloadEvent} */
                ^

0 error(s), 1 warning(s)

That's strange, since the following executed in the browser:

onbeforeunload = function(e) {
    console.log(e);
    e.preventDefault();
    return "";
};

returns BeforeUnloadEvent in the console. Why does GCC not support it?

supahero1 avatar Aug 16 '22 18:08 supahero1

It looks like the browser externs included in closure are missing this. For example, if such a type was declared, this line could be changed to handle that event: https://github.com/google/closure-compiler/blob/6755ad8946bb9dec238dab2e135854e873da7f26/externs/browser/w3c_dom1.js#L726

https://developer.mozilla.org/en-US/docs/Web/API/BeforeUnloadEvent suggests that this is well supported, and probably would be reasonable to include?

With that said, I don't think most browsers support the returnValue field any longer, so functionally just Event might be enough for most purposes?

niloc132 avatar Aug 16 '22 20:08 niloc132

https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event

concavelenz avatar Aug 24 '22 15:08 concavelenz

I don't think that not adding the event type is wise just because it has poor support on mobile. On desktops it works and is great for displaying the "Are you sure you want to leave?" alert. Take browser games as an example. You can play them for hours in one session, and one missclick can end the entire progress you've made. The unload event is very beneficial then to prevent a disaster. Besides, you support MouseEvent and other "rather desktop-specific" events (yes I am aware they can also occur on mobiles but that's a subset), while there's the new PointerEvent API. By your logic, will you remove support for other events now, that PointerEvent is more universal? Don't be joking me. Please add the BeforeUnloadEvent.

supahero1 avatar Aug 26 '22 11:08 supahero1