jsduck
jsduck copied to clipboard
senchalabs
Support alternative Ext.define syntax
Ext JS 4 supports and documents two primary syntaxes for defining classes. See the documentation here: http://docs.sencha.com/extjs/4.2.0/#!/api/Ext-method-define
JSDuck has special support for the first syntax. It understands properties like extends, mixins, and singleton. Undocumented methods are automatically included in the API docs as private methods.
Unfortunately, JSDuck does not have any of this special support second Ext.define syntax.
We have a very large code base that is using the second syntax (because it allows us to create a block scope that encapsulates our classes code).
Here are the examples from the official Ext JS 4.2 documentation:
Syntax 1:
Ext.define('My.awesome.Class', {
someProperty: 'something',
someMethod: function(s) {
alert(s + this.someProperty);
}
});
Syntax 2:
Ext.define('MyApp.foo.Bar', function () {
var id = 0;
return {
nextId: function () {
return ++id;
}
};
});
I didn't find a way to attach a test case, so here are two versions of a class that a almost identical, but produce very different output.
Syntax 1
/**
* one class that does something
*/
Ext.define('Class1', {
extend: 'Ext.window.Window',
/**
* do something cool
*/
methodWithDoc: function (a) {
return a;
},
methodWithoutDoc: function (a) {
return a;
}
});
Syntax 2
/**
* a second class
*/
Ext.define('Class2', function () {
return {
extend: 'Ext.window.Window',
/**
* do something cool
*/
methodWithDoc: function (a) {
return a;
},
methodWithoutDoc: function (a) {
return a;
}
};
});
Syntax 1 Ouput
Syntax 2 Ouput
That's a fundamental problem with no easy workaround.
The function, that's passed to Ext.define can possibly do loads of complicated stuff which JSDuck can't understand by static analyzes alone. Like there could be an if statement which returns either one or the other configuration object.
However, I guess things can be improved to cover a simple case where there's just one return statement with an object literal.
Thanks for the quick rely.
That would be great if jsduck could cover just the "simple case" of a single return statement with a single object literal (as shown in the official Ext JS API docs). Ideally, jsduck should treat the object literal returned by the function the exact same as you treat the direct object literal.
Thanks
These changes work for me (JSDuck 5.2.0):
lib/jsduck/js/class.rb (line 106, "detect_ext_define(..)")
cls[:code_type] = :ext_define
- ast["arguments"][1].each_property do |key, value, pair|
+ dnode = ast["arguments"][1]
+
+ # Support both function and object notation
+ if dnode.function_expression?
+ dnode = dnode.return_statement_expression
+ return if dnode.nil?
+ end
+
+ dnode.each_property do |key, value, pair|
if tag = TagRegistry.get_by_ext_define_pattern(key)
lib/jsduck/js/node.rb (multiple locations)
+ # Returns the return statement's expression for the current function
+ # expression.
+ def return_statement_expression
+ return unless function_expression?
+
+ node = child("body")
+ if node.block_statement?
+ node["body"].each do |node|
+ if node.return_statement?
+ node = node["argument"]
+ return node.object_expression? ? node : nil
+ end
+ end
+ end
+
+ return nil
+ end
+
+
+ def block_statement?
+ @node["type"] == "BlockStatement"
+ end
+
+ def return_statement?
+ @node["type"] == "ReturnStatement"
+ end
@pgleichmann, I would love to test your fix. Would you mind attaching your complete node.rb file? Thanks.
Thanks @pgleichmann. The fix works for the simple case like:
/** */
Ext.define('MyClass', function() {
return {extend: "Your.Class"};
});
But it fails for code like:
/** */
Ext.define('MyBox', function() {
if (isIE) {
return {extend: "TableBox"};
}
return {extend: "Html5Box"};
});
Reporting that MyBox extends Html5Box although it might just as well extend the TableBox class. Sure, that's a weird corner case, but I'd like to be sure, not just assume. So I'll need to do some additional work on this change before pulling it in.
@nene I understand the corner case and we do have 1 class like that. However, we have ~700 classes that match the "simple case". I'm personally not worried about the corner case. I'm not sure how jsduck should even handle that.
It would be a huge help to several teams I work with to have @pgleichmann's fix accepted even though it only handles the simple/common case.
Thanks
@mattsorensen Since Github does not allow to attach files to issues, I've created a Gist.
@nene It is true that the patch only for for simple function with a single return statement. I primarily use the function notation to keep definitions private.
Ext.define('App.util.Example', function() {
var SUPER_SECRET_RE = /.../;
return {
testIt: function(str) {
return SUPER_SECRET_RE.test(str);
}
};
});
As for your IE example, I personally would create an abstract class/interface and then create two sub-classes.
@pgleichmann I just applied your patch and tested on 450+ classes. It works GREAT. Thanks for sharing. Hopefully it can be pulled into the official build soon.
Hi, it's been a while... I've been a bit away from JSDuck.
I've pulled this patch in.
Hopefully I can also make the next release happen soon enough.
Ideally (I know it's not easy) JSDuck can support any class forms that are officially supported by Sencha Cmd, including the internal IIFE:
Ext.define('App.namespace.Class', (function () {
return {
extend: 'Ext.Base'
};
}()));
Previously we wrapped our defines in an outer IIFE to make them work with both Sencha Cmd and JSDuck, but unfortunately that door has been closed. The outer style now breaks Sencha Cmd 5.
I have a fix for the IIFE, building on to the prior commit for functions. I signed the contributor waiver, still waiting to hear back and get permission. Would you like me to share in another manner?
Apologies, I've tried repeatedly to push to my fork through our corporate proxy but it seems to not be possible. I'm sure you would prefer to pull, but I just can't seem to get around this. I've created a Gist that includes the functional change and tests. I hope that is sufficient.
If you don't mind Don, I'll grab your changes submit a pull request for you.
Hi Ben, As you pointed out, original GIST mistakenly had the code from lib\jsduck\class.rb instead of lib\jsduck\js\class.rb. I've updated the GIST with the correct code. Apologies for the confusion.