Leaflet / Leaflet

🍃 JavaScript library for mobile-friendly interactive maps 🇺🇦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.17k stars 5.75k forks source link

Should event-listeners stop executing code? #9288

Open Falke-Design opened 2 months ago

Falke-Design commented 2 months ago

Checklist

Steps to reproduce

If we fire a event, the code after fireing will be not executed, if the listener throws an error:

map.on('customEvent',()=>{
    throw new Error(`I don't like that event!`)
})

function fireSomeEvent() {
    console.log('Some task ...')

    map.fire('customEvent');

    console.log('Another task ...')
}
fireSomeEvent()

Expected behavior

I'm not sure about what is right, but I have expected that the fireSomeEvent function will not break and continue executing.

Current behavior

fireSomeEvent function breaks after an Error is thrown in the listener of the fired event

Minimal example reproducing the issue

https://plnkr.co/edit/apIUNihbz3VCAAia

Environment

IvanSanchez commented 2 months ago

Compare with the behaviour of the built-in EventTarget:

const foo = new EventTarget();

foo.addEventListener('customEvent',()=>{
    throw new Error(`I don't like that event!`)
})

function fireSomeEvent() {
    console.log('Some task ...')

    const ev = new CustomEvent('customEvent');
    foo.dispatchEvent(ev);

    console.log('Another task ...')
}
fireSomeEvent()

This will log "Another task".

We never really set the Leaflet Evented behaviour on stone, but I guess that @Falke-Design is right - the behaviour is not as expected (since I'd expect the same behaviour than EventTarget).

I'd even say "let's replace Evented with a subclass of EventTarget", since it's 2024 and all browsers implement it.