Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#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/trunk@22254) 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)

no-new.diff (4.9 KB) - added by bradfordcarlsmith 9 years ago.
Patch for dojo.declare() to eliminate need for 'new' when creating objects

Download all attachments as: .zip

Change History (11)

comment:1 Changed 9 years ago by James Burke

Owner: changed from anonymous to Eugene Lazutkin

comment:2 Changed 9 years ago by Eugene Lazutkin

Milestone: tbdfuture
Status: newassigned

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.

comment:3 Changed 9 years ago by bradfordcarlsmith

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/trunk@22464.

comment:4 in reply to:  2 Changed 9 years ago by bradfordcarlsmith

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 9 years ago by Eugene Lazutkin

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 9 years ago by Eugene Lazutkin

Hmm, maybe it does... In any case it should be added to the unit test.

Changed 9 years ago by bradfordcarlsmith

Attachment: no-new.diff added

Patch for dojo.declare() to eliminate need for 'new' when creating objects

comment:7 Changed 9 years ago by bradfordcarlsmith

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/trunk@22464 just like the last one.

comment:8 Changed 9 years ago by Eugene Lazutkin

Milestone: future1.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 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: assignedclosed

(In [22473]) Added checks for constructor being called without "new", thx bradfordcarlsmith!, !strict, fixes #11237.

comment:10 Changed 7 years ago by bill

In [28058]:

Fix parentheses; test was inadvertently working due to #11237. Refs #11237.

Note: See TracTickets for help on using tickets.