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

Implement legacyOutputDidListenersThrowFlag #36

Open
bigeasy opened this issue Jan 26, 2021 · 2 comments
Open

Implement legacyOutputDidListenersThrowFlag #36

bigeasy opened this issue Jan 26, 2021 · 2 comments

Comments

@bigeasy
Copy link

bigeasy commented Jan 26, 2021

There's an argument to dispatchEvent that, if given, will set a legacyOutputDidListenersThrowFlag if a user given callback raises an exception. It was added for IndexedDB which had been monkey patching the event mechanism to determine if a user given callback raised an exception in order to roll back a transaction if it did. This feature is only used by IndexedDB, so it makes sense that no one has implemented it in their event shims, but I'm having a go at implementing IndexedDB and it would be nice to have.

I believe it would be as simple as setting the property in the catch block of the try block where you invoke the user given callback for an event target. I'll be running tests agains the W3C integration suite for IndexedDB and will be able to report on whether or not the implementation in target-event-shim works correctly. I'm willing to submit a patch if you're pressed for time.

The discussion of legacyOutputDidListenersThrowFlag begins here with "If an exception was propagated out from any event handler" is not a thing. The description of the algorithm can be found in the DOM standard under 2.9. Dispatching events.

Additional points of reference the discussion under Add legacyOutputDidListenersThrowFlag to event dispatch for IDB at the repo for the DOM spec. The PR that updated the IndexedDB spec for legacyOutputDidListenersThrowFlag. Tests for the proper behavior are part of the IndexedDB integration test suite.

@mysticatea
Copy link
Owner

Thank you for your proposition.

We cannot add extra arguments to EventTarget.prototype.dispatchEvent method, but can add a new function.

import { EventTarget, dispatchEvent } from "event-target-shim"

const target = new EventTarget()
const context = { legacyOutputDidListenersThrowFlag: false }

// dispatch with context
dispatchEvent(target, new Event("foo"), context)

console.log(context.legacyOutputDidListenersThrowFlag)

@bigeasy
Copy link
Author

bigeasy commented Feb 4, 2021

Thank you for taking an interest in this issue. Now that you mention it, I see that the interface for dispatchEvent does not include parameters for legacyOutputDidListenersThrowFlag but the algorithm as defined in the DOM standard does. That's curious. I understand that you cannot change dispatchEvent. When I look at the ]jsdom implementation of EventTarget](https://github.com/jsdom/jsdom/blob/5279cfda5fe4d52f04b2eb6a801c98d81f9b55da/lib/jsdom/living/events/EventTarget-impl.js#L104) I can see that they have a private function that accommodates the legacy target override flag and makes mention of legacyOutputDidListenersThrowFlag. (Sorry, I'm kind of talking to myself here. I'm sure you know a lot more about these interfaces and implementations than I do.)

Allow me to re-read the notes I gathered and report back on any IndexedDB specifics I might find. I believe I can get a first pass implementation using the setErrorHandler function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants