Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#10829 closed defect (fixed)

[cla][patch] dojo.declare doesn't warn when a parent class doesn't exist on declaration.

Reported by: Tom Trenka Owned by: Eugene Lazutkin
Priority: high Milestone: 1.5
Component: Core Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

Ran across an issue where, in a large-ish application, with dojo.requires all over the place, a class was being declared based on a parent class, but that parent class hadn't been loaded/required yet. This ended up causing a bit of pain in terms of debugging, because the behavior exhibited as a result was entirely unexpected (this was a largish widget chain, and trying to create a new instance of that widget returned an object but did not fire any of the expected widget lifecycle methods).

I'm wondering if it'd be possible/prudent to give a console.warn during the ctor setup when a specified parent class is actually undefined? It would help the debugging process a lot, and should be stripped out on a build anyways.

Thanks in advance-- Tom

Attachments (1)

c3mro-10829 Diff.txt (1.5 KB) - added by mtyson 9 years ago.
Diff of error handling for c3mro when proto is undefined

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by Eugene Lazutkin

Milestone: tbdfuture
Status: newassigned

comment:2 Changed 9 years ago by Eugene Lazutkin

Component: GeneralCore

comment:3 Changed 9 years ago by Eugene Lazutkin

Milestone: future1.5

comment:4 Changed 9 years ago by mtyson

Here's a diff of some error handling here. I found some cases where base.declaredClass is undefined in c3mro(), so I throw and catch the error in declare() where the superclass is known.

Changed 9 years ago by mtyson

Attachment: c3mro-10829 Diff.txt added

Diff of error handling for c3mro when proto is undefined

comment:5 Changed 9 years ago by Eugene Lazutkin

Summary: dojo.declare doesn't warn when a parent class doesn't exist on declaration.[cla][patch] dojo.declare doesn't warn when a parent class doesn't exist on declaration.

comment:6 Changed 9 years ago by Eugene Lazutkin

Priority: normalhigh

comment:7 in reply to:  4 Changed 9 years ago by Eugene Lazutkin

Replying to mtyson:

Here's a diff of some error handling here. I found some cases where base.declaredClass is undefined in c3mro(), so I throw and catch the error in declare() where the superclass is known.

Your patch assumes that the base is a valid object, yet its prototype is missing/undefined. How it can happen? The only way to do it is to pass a non-function object as a base, which is clearly wrong (see docs).

I need a test case demonstrating the problem.

comment:8 Changed 9 years ago by Eugene Lazutkin

(In [21794]) Better troubleshooting in the case of missing base classes, stricter tests, !strict, refs #10829.

comment:9 Changed 9 years ago by Eugene Lazutkin

Priority: highnormal

This ticket is not closed yet because I am waiting for a test case.

comment:10 Changed 9 years ago by Tom Trenka

The case I ran across was when something referencing an object declared in one file was being required before the object was actually declared (i.e. circular reference). For example...

dojo.provide("myWidgets.Toolbar");
dojo.require("myWidgets.SpecializedToolbar");

dojo.declare("myWidgets.Toolbar", null, { ... });

And then in SpecializedToolbar?...

dojo.provide("myWidgets.SpecializedToolbar");

dojo.declare("myWidgets.SpecializedToolbar", myWidgets.Toolbar, { ... });

What was happening for me was that SpecializedToolbar? was not ctor'd correctly, because of the bad require (as you said, the base class should not be an object). But it was difficult to track down why, which is why I was asking for maybe a warning or something...in this particular case, the base class was already created as an object (because of the provide) but was not yet declared when the subclass was defined.

Hope that helps? Perhaps just checking to see if the base class/mixins are actually functions would do just fine...

comment:11 Changed 9 years ago by Eugene Lazutkin

Got it, thx Tom.

comment:12 Changed 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: assignedclosed

(In [21798]) Added Tom's case, when base is an automatically created object, !strict, fixes #10829.

comment:13 Changed 9 years ago by mtyson

Eugene said: "Your patch assumes that the base is a valid object, yet its prototype is missing/undefined. How it can happen? The only way to do it is to pass a non-function object as a base, which is clearly wrong (see docs)."

We were seeing this problem when a class had a superclass that wasn't required by the class file. It was not consistently repeatable. It depended on timing, and was more prevalent in XD loading. Essentially, it would arrive in c3mro() and proto would be undefined.

The issue was in a large app if someone creates this situation, it could take a very long time to find where the problem lay with the given error message (and the person would first have to determine what the problem was, because the error message was '_f5 is undefined' for XD and 'proto is undefined' for regular loading).

In any case, the error handling was designed to make it clear what class was attempting to use an undefined superclass (and thereby make it clear what file was missing a require()).

Not sure if your patch addresses this case or not.

comment:14 in reply to:  13 Changed 9 years ago by Eugene Lazutkin

Replying to mtyson:

Not sure if your patch addresses this case or not.

It does, but don't take my word for it --- test it with your use cases or applications to see if it works for you. The more testing we get while in beta the better.

Note: See TracTickets for help on using tickets.