prototypejs / prototype

Prototype JavaScript framework
http://prototypejs.org/
Other
3.54k stars 639 forks source link

Gracefully handle missing elements #296

Open jwestbrook opened 9 years ago

jwestbrook commented 9 years ago

Scenario

document.observe('dom:load',function(){

   $('div1').update('my div')

});

If the element div1 does not exist in the DOM then the js errors and nothing else past that line is ran. The error ends up being cannot read property of null which is not descriptive. (could be different in different browsers but still not the actual problem)

so question is

  1. This is expected behavior, devs should wrap the statements in if blocks
  2. We should throw a better error, "Element with id(s) div1 does not exist"
  3. Gracefully fail, or return an empty element to prevent the JS error and throw a warning

This applies to Element#down() as well

savetheclocktower commented 9 years ago

This is actually my least favorite thing about jQuery — you can call methods on an empty result set without knowing it's empty. My feeling is that the user will generally want to know if div1 does not exist, because that violates expectations.

In the cases where the user is OK with div1 not existing, then we could introduce something like the Maybe monad. (Though something like that would likely need to rely on ES6 proxies to work.)

Option #2 is interesting, but I'm on the fence about whether it's overkill, since most browsers have good developer tools these days. It's not like the days of IE6 when you had a cryptic error and couldn't trace it back to the proper line number.

jwestbrook commented 9 years ago

I agree that I want to know that an element is missing, sometimes tracking it in minified or uglified code is difficult though and the custom throw would be useful to narrow it down what $() is barfing.

Currently its a cryptic error anyway so throwing a specific error would be an improvement, with minimal effort, and match expectations - ie if I give $() a bad id it will tell me, instead of surprise I get an error about not being able to read a property.

I know you are close to getting the next bugfix out - just wanted to see if a concern one of the devs I know could be addressed.

savetheclocktower commented 9 years ago

This might be worth doing when a single string is passed into $ just to make debugging a bit easier. I'll put it in the possible enhancements list.