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

Should event-listeners stop executing code? #9288

Open
4 tasks done
Falke-Design opened this issue Mar 6, 2024 · 1 comment
Open
4 tasks done

Should event-listeners stop executing code? #9288

Falke-Design opened this issue Mar 6, 2024 · 1 comment
Labels
bug needs triage Triage pending

Comments

@Falke-Design
Copy link
Member

Checklist

  • I've looked at the documentation to make sure the behavior isn't documented and expected.
  • I'm sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
  • I've searched through the current issues to make sure this hasn't been reported yet.
  • I agree to follow the Code of Conduct that this project adheres to.

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

  • Leaflet version: 1.9.4
  • Browser (with version): Firefox
  • OS/Platform (with version): Windows 10
@Falke-Design Falke-Design added bug needs triage Triage pending labels Mar 6, 2024
@IvanSanchez
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs triage Triage pending
Projects
None yet
Development

No branches or pull requests

2 participants