search

observablehq / stdlib

The Observable standard library.
https://observablehq.com/@observablehq/standard-library
ISC License
966 stars 83 forks source link

Error on invalid dispose function. #162

Closed mbostock closed 4 years ago

mbostock commented 4 years ago

Fixes #89.

My initial inclination was to just log a warning, but since warnings are only visible if you open the developer console, I figured that wouldn’t actually help people recognize the mistake.

visnup commented 4 years ago

For the example linked in #89 this would instead immediately error for that cell with "invalid dispose"? Is it possible to give a hint to the mechanism for the error in the message?

mbostock commented 4 years ago

Saying that the returned dispose function is invalid is a hint, no?

We could have an error message that specifically targets promises by looking for a then-able if you think that’s worth the trouble.

visnup commented 4 years ago

Re-reading the README docs on Generators.observe which does document the dispose return function, so it could be enough, but I don't think it's obvious that the return value is always consumed that way, especially when mixing with async.

I was thinking of the warning you get if you try to use an async function inside of useEffect: