Search and Top Navigation
#5972 closed enhancement (fixed)
Opened August 18, 2010 11:05PM UTC
Closed December 10, 2010 07:12PM UTC
Widget: Throw error for non-existent method calls
Reported by: | keturn | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.9.0 |
Component: | ui.widget | Version: | 1.8.4 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
$( el ).plugin( invalidMethod ) should throw an exception. Calling a method before initialization should throw an exception as well.
Attachments (0)
Change History (14)
Changed August 19, 2010 12:17AM UTC by comment:1
component: | ui.autocomplete → ui.widget |
---|---|
description: | I recently had a line of code that did this: \ \ ed.autocomplete('options', {source: datasource}); \ \ There were two things wrong with this code: \ \ 1) The name of the autocomplete method to set an option is 'option', not the plural 'options'. \ \ 2) Turns out that the "ed" element never had an autocomplete instance initialized on it in the first place, so trying to set options for it was in error. \ \ However jQueryUI didn't issue any sort of alert, warning, or error that I was doing something totally nonsensical. → $( el ).plugin( invalidMethod ) should throw an exception. Calling a method before initialization should throw an exception as well. |
summary: | should issue warning or error when invoking the wrong method → Widget: Throw error for non-existant method calls |
Changed August 19, 2010 09:21AM UTC by comment:2
summary: | Widget: Throw error for non-existant method calls → Widget: Throw error for non-existent method calls |
---|
Changed August 19, 2010 09:53AM UTC by comment:3
I'm as hesitant to implement this as I would be to throw an exception
- if a method receives an argument of a different type than is expected
- if a method receives a different number of arguments than expected
- if an option is set/get that the original plugin author is not aware of
These provides no benefit at run-time, only at development/debug time and all have serious potential to limit extensibility.
We can draw a parallel (even if stretched) to the oft-requested "feature" that jQuery Core throw an exception if any jQuery method is called against an empty set. That is surely not an exception, but in certain circumstances people feel like they would have had time saved if they were made aware of its occurrence. And while I recognize such a direct comparison is a stretch (closer would be calling an undefined method in JavaScript, which is an exception), I'm careful to preserve what we may have currently as powerful rather than dangerous.
Just as an example, declaring it an exception to call a non-existent widget method would preempt someone from creating a noSuchMethod implementation for jQuery UI widgets, something that is currently a real possibility, and perhaps even interesting.
If this is so sorely needed at development/debug time, couldn't it simply be opted in via a debug plugin? Such a debug plugin would be very easy to write if we were to build in a noSuchMethod hook but difficult (if even possible) to write if we declared such a function call an exception.
Changed August 19, 2010 12:11PM UTC by comment:4
I'm also hesitant to add any throwing at such a high level. When I saw the ticket, my first thought was to actually create a noSuchMethod method and allow that to handle it however you want. The default implementation could just be an empty function. As for calling methods on uninstantiated plugins, we may decide to auto-instantiate with default options in the future.
Changed August 19, 2010 09:14PM UTC by comment:5
Well, there's clearly some background information I'm missing, because I don't understand why plugin methods go like $(selector).widget('method') instead of $(selector).widget.method() like the rest of jQuery and JavaScript, but, as for this:
Just as an example, declaring it an exception to call a non-existent widget method would preempt someone from creating a noSuchMethod implementation for jQuery UI widgets, something that is currently a real possibility, and perhaps even interesting.
This doesn't seem to be a problem for Python or Ruby. They have no problems with giving me an error when I try to invoke a method that doesn't exist, but offer __getattr__
and method_missing
if I want another behavior.
My expectation as a developer is that a method is going to behave like a JavaScript method, and the JS engines I use will give me an error if I try to call something that's undefined. Don't break that expectation for a "someday we might" feature. And we don't need *more* ways to shoot ourselves in our foot.
Changed August 27, 2010 06:49PM UTC by comment:6
milestone: | TBD → 1.9 |
---|---|
resolution: | → fixed |
status: | new → closed |
Fixed in 1e28040.
Changed September 10, 2010 05:28PM UTC by comment:7
milestone: | 1.9 → 1.8.5 |
---|
Changed October 20, 2010 04:21PM UTC by comment:8
resolution: | fixed |
---|---|
status: | closed → reopened |
Hey Scott -
I see you commented 2 months ago that you did not want to throw an exception so high up, which I agree with. Then it had a milestone for 1.9, the next major release, which is debatable in itself. Then it somehow got into the minor release of 1.8.5 therefore being the first UI minor release that has broken my application. For example, the upgrade guide only says 1.7 to 1.8. There is no warning of a harmless return false changing to throwing an exception in ui.widget.
I'm definitely on board with an exception for a non-existent method because that is standard - you can't call what doesn't exist at all ever. Throwing an exception before something is initialized seems to me like making $('#notonpageyet').height() throw an exception.
What you guys do is great and I don't mean to sound like a prick, but I'm wondering if you can shed some light on what happened and why. Thanks.
Changed October 20, 2010 09:14PM UTC by comment:9
resolution: | → fixed |
---|---|
status: | reopened → closed |
Please use the forums for discussions.
Changed October 20, 2010 09:28PM UTC by comment:10
Ironic since I started there and headed here since I found this as the source of problem :) Here is the post. Sorry about that... http://forum.jquery.com/topic/jqueryui-1-8-5
Changed October 21, 2010 01:54PM UTC by comment:11
milestone: | 1.8.5 → 1.9 |
---|---|
resolution: | fixed |
status: | closed → reopened |
Reverted in 6ba75aa. Going to add back in for 1.9.
Changed November 19, 2010 06:26PM UTC by comment:12
resolution: | → fixed |
---|---|
status: | reopened → closed |
Widget: Throw errors when calling non-existent methods or methods on uninistantiated widgets. Fixes #5972 - Widget: Throw error for non-existent method calls.
Changeset: 1e28040cf358d0fe81ee83a2e35d7dbb65362afa
Changed November 19, 2010 06:50PM UTC by comment:13
resolution: | fixed |
---|---|
status: | closed → reopened |
Changed December 10, 2010 07:12PM UTC by comment:14
resolution: | → fixed |
---|---|
status: | reopened → closed |
Widget: Throw errors for invalid method calls. Fixes #5972 - Widget: Throw error for non-existent method calls.
Changeset: 9ad2a4b1ccebb32cc745be3ef85a4b634e416ff8