#11237 closed enhancement (fixed)
dojo.declare(): return functions that don't require 'new'
Reported by: | bradfordcarlsmith | Owned by: | Eugene Lazutkin |
---|---|---|---|
Priority: | high | Milestone: | 1.5 |
Component: | Core | Version: | 1.5.0b2 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
If a constructor function (class) is accidentally called without the 'new' operator it will get the global object as 'this', so it will modify the global object in possibly bad and definitely ugly ways.
It's relatively easy, though, to define a constructor function that will detect when it is called without 'new' and behave is if the 'new' were there by creating a new object with the appropriate prototype, invoking itself on that object, then returning the object. As a bonus, if you do this regularly, then you can eliminate the string 'new ' all over your code.
The attached diff (created against browser:/dojo/[email protected]) contains minor changes to dojo.declare() to make the functions it returns behave this way. It also includes unit tests for the new functionality. I hope you will consider incorporating it into dojo. I emailed my CLA a few hours ago.
Attachments (1)
Change History (11)
comment:1 Changed 11 years ago by
Owner: | changed from anonymous to Eugene Lazutkin |
---|
comment:2 follow-up: 4 Changed 11 years ago by
Milestone: | tbd → future |
---|---|
Status: | new → assigned |
comment:3 Changed 11 years ago by
Just realized that my patch had a bug. Instead of checking to see whether 'this' is the global object, I should use instanceof to see if it has the correct prototype. Checking against the global object won't work if the constructor is called like this.
var foo = my.namespace.FooClass();
In this case FooClass() will get passed my.namespace as its 'this' object.
I've fixed this problem and uploaded a new patch made against browser:/dojo/[email protected].
comment:4 Changed 11 years ago by
Replying to elazutkin:
While it is relatively easy to do, I am not so sure that we want to "enhance" JavaScript with Python-like way of creating new objects. It'll lead to a confusion. But it appears to be a good practice to check against newb's mistakes.
OK. I would like to point out, though, that core JavaScript already contains constructor functions that can safely be called without 'new'. They either behave as if the 'new' were there (e.g. Array, Error, and Object) or vary their behavior somewhat (e.g. Boolean, RegExp, and Date) in order to avoid the default "modifies-the-wrong-object-and-returns-undefined" behavior.
Thanks for taking a look at my code anyway.
comment:5 Changed 11 years ago by
What about the case of mixins?
var B = dojo.declare([A, M, N], {}); B instanceof A; // true B instanceof M; // false B instanceof N; // false
It doesn't appear that the patch handles this case.
comment:6 Changed 11 years ago by
Hmm, maybe it does... In any case it should be added to the unit test.
Changed 11 years ago by
Attachment: | no-new.diff added |
---|
Patch for dojo.declare() to eliminate need for 'new' when creating objects
comment:7 Changed 11 years ago by
I've uploaded a new version of the patch containing additional unit tests to verify correct behavior for multiple inheritance and when calling a constructor through an object (e.g. foo.bar.MyClass?() instead of just MyClass?()). This version was created against browser:/dojo/[email protected] just like the last one.
comment:8 Changed 11 years ago by
Milestone: | future → 1.5 |
---|
Looks okay, but the cleanup is not done correctly --- I'll take care of it myself.
The patch is of low impact and will help with hard to find bugs made by newbies, when global objects are modified accidentally. I'll commit it to 1.5.
comment:9 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
While it is relatively easy to do, I am not so sure that we want to "enhance" JavaScript with Python-like way of creating new objects. It'll lead to a confusion. But it appears to be a good practice to check against newb's mistakes.