Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

"Extends: undefined" (or equivalent) is unhandled #2257

Closed
giddie opened this Issue · 17 comments

5 participants

@giddie

It took me quite a while to figure out what was causing this issue for me, but when I have two classes in two separate files, and one depends on the other, I may end up with this situation:

First = new Class({
});

Second = new Class({
  Extends: First
});

If Second is accidentally loaded before First, the Extends property exists, but is undefined. This causes issues through the following chain of line numbers in Class.js: 29 (implement) -> 77 (key = 'Extends', value = undefined) -> 104 -> 92, and we get a "klass is undefined" error.

I suggest that it would be a good idea to handle this more gracefully. The best thing would probably be to throw a more descriptive error message, so that it's clearer to the developer that the issue is with the file load order.

@cpojer
Owner

Unfortunately this is a JavaScript error and there is nothing we can do about this. If you are using Web Inspector you can get much more useful error messages, open Web Inspector (Chrome or Safari) in this example: http://jsfiddle.net/M6efP/ and you will see detailed error text.

You should consider integrating dependency management so your files will always be included in the proper order. See http://requirejs.org/ or our own http://github.com/kamicane/packager

@cpojer cpojer closed this
@ibolmo
Owner

Yeah JavaScript engines have never done a great job for helping the developer, but that's the derivative of having a runtime language. Worst that JS is so easy to manipulate e.g. (undefined =).

Your suggestion is valid, but not solvable in the near future. Perhaps in 1.5 or 1.6. I'll slate this for 1.5 and we can move it to 1.6. The solution is to use:

if (has('dev')) throw new Error('Cannot extend child class with an undefined parent class.');

This requires has support, which #2258 addresses (copied from Milk issues repo).

@ibolmo ibolmo reopened this
@cpojer
Owner

@ibolmo I don't understand how has.js would solve the issue of referencing undefined variables unless we switch to string based class names and a general class namespace.

@cpojer cpojer closed this
@timwienk
Owner

What the pojer said: http://jsfiddle.net/M6efP/1/

@cpojer
Owner

Are we playing this game now? http://jsfiddle.net/M6efP/2/

@ibolmo ibolmo reopened this
@ibolmo
Owner
var method = function(child, parent){
  if (has('dev') && (parent == null)) throw new Exception('Cannot extend if
 parent class does not exist.');
  // ...
};

method(new Class(), this['doh']);

get it yet?

@cpojer
Owner

nope.

@ibolmo
Owner

I see @cpojer's and @timwienk's point. In the Extends: Case the JS parser will throw exception for RefereceErrors.

My example (updated) is about initializing options or arguments with undefined variables that are required for initialization of the class.

I'm closing the issue. :D

@ibolmo ibolmo closed this
@seanmonstar

True, in his example, it would throw a reference error, but here's an example that wouldn't:

Namespace.B = new Class({

    Extends: Namespace.A

});

And MooTools does this a lot (Fx, Drag, Request, etc).

@ibolmo
Owner
@seanmonstar
@ibolmo
Owner
@seanmonstar
@seanmonstar
@giddie

I don't get what the problem is with this suggestion. It seems to me that a meaningful error message that clarifies the high-level issue is pretty much by definition better than an obscure reference error further down the stack that takes hours in a debugger picking apart Mootools to discover is actually just a stupid load-order issue.

Surely the point of Mootools is to make Javascript awesomer, and I can't think of any way awesome could be defined as breakage in an anonymous function when the developer tries to extend with an undefined class. Surely this same thing will be an issue if the developer just mistypes a class name in the Extends property?

@cpojer
Owner

@giddie the issue is that when you reference a variable that is not defined, execution of the file or function halts. If you do new Class({ Extends: SomeUndefinedVariable }), the "new Class" constructor does not get called at all because your JS engine fails to resolve SomeUndefinedVariable. Also see this example: http://jsfiddle.net/M6efP/2/ which will throw an error in your console.

If this happens, we cannot intercept this error and we are unable to throw a useful custom error message.

As I said in my first response, use Web Inspector and you'll get a very useful error message with file and line that helps you more than we ever could in a custom error. If your preferred debugger does not give you good output, file a bug with them describing how web inspector has better error texts. Thank you.

@seanmonstar seanmonstar referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@seanmonstar seanmonstar referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@seanmonstar seanmonstar referenced this issue from a commit in seanmonstar/mootools-core
@seanmonstar seanmonstar fixes #2257 - give a better error if the value for Extends or Impleme…
…nts isn't valid
76eacf3
@giddie

@cpojer I don't think you understand my problem. In the situation I described, neither Ffx not Chrome picked up on my symbol being undefined, and I ended up with an obscure "klass is undefined" much further down the stack. As @seanmonstar demonstrated, it was probably actually because the class was being assigned to an object property, instead of directly to a new object. I didn't think to check for a distinction when I wrote my first post.

@seanmonstar Those changes look like exactly what I was hoping for; nice one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.