From 4e1380837f07756ac0ffdc90f77a7ab8b45d1c8b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 9 Dec 2011 23:07:41 -0500 Subject: [PATCH] Began making changes ... (overwrite this commit) --- Makefile | 6 ++++- README.hacking | 9 +++++++ lib/ClassBuilder.js | 14 +++++------ lib/MemberBuilderValidator.js | 2 +- lib/util.js | 8 +++--- test/Interface/ExtendTest.js | 8 +++--- test/MemberBuilderValidator/MethodTest.js | 15 +++++++++--- test/MemberBuilderValidator/PropertyTest.js | 11 +++++++-- test/MemberBuilderValidator/inc-common.js | 2 +- test/inc-testcase.js | 27 ++++++++++++++++++--- test/test-class-abstract.js | 6 ++--- test/test-class-constructor.js | 13 +++++----- test/test-class.js | 6 +++-- test/test-combine-pre-es5.js | 3 ++- test/test-combine.js | 9 ++++--- tools/combine | 2 +- tools/combine-test.tpl | 6 +++-- tools/combine.tpl | 14 ++++++----- 18 files changed, 110 insertions(+), 51 deletions(-) diff --git a/Makefile b/Makefile index 9b0da39..70741d7 100644 --- a/Makefile +++ b/Makefile @@ -64,7 +64,11 @@ $(compiler): | tar -C $(path_tools) -xzv compiler.jar build/%.min.js: build/%.js $(compiler) cat $(path_tools)/license.tpl > $@ - java -jar $(compiler) --js $< >> $@ || rm $@ + java -jar $(compiler) --compilation_level ADVANCED_OPTIMIZATIONS \ + --formatting pretty_print \ + --property_map_output_file build/min-propmap.xml \ + --warning_level VERBOSE \ + --js $< >> $@ || rm $@ install: doc-info [ -d $(path_info_install) ] || mkdir -p $(path_info_install) diff --git a/README.hacking b/README.hacking index 7254909..f738420 100644 --- a/README.hacking +++ b/README.hacking @@ -30,6 +30,15 @@ Under no circumstances should a test be completely removed without being replaced unless the feature was entirely removed. +Compiling +--------- +Closure Compiler will rename all unquoted properties. It is therefore important +to keep consistent with how properties are accessed. All properties that are to +remain accessible by name when minified must be quoted. Furthermore, all +properties defined using defineSecureProp() or by means of any other string +definition (e.g. Object.defineProperty()) must be accessed as a quoted string. + + Submitting Patches ------------------ You may send pull requests via your preferred method or e-mail patches to the diff --git a/lib/ClassBuilder.js b/lib/ClassBuilder.js index 73f545c..82ec57f 100644 --- a/lib/ClassBuilder.js +++ b/lib/ClassBuilder.js @@ -247,7 +247,7 @@ exports.isInstanceOf = function( type, instance ) // if no metadata is available, then our remaining checks cannot be // performed - if ( !instance.__cid || !( meta = exports.getMeta( instance ) ) ) + if ( !instance['__cid'] || !( meta = exports.getMeta( instance ) ) ) { return false; } @@ -613,7 +613,7 @@ exports.prototype.createConcreteCtor = function( cname, members ) } initInstance( this ); - this.__initProps(); + this['__initProps'](); // If we're extending, we don't actually want to invoke any class // construction logic. The above is sufficient to use this class in a @@ -746,7 +746,7 @@ exports.prototype._attachPropInit = function( // defaults to false inherit = !!inherit; - var iid = this.__iid; + var iid = this['__iid']; // first initialize the parent's properties, so that ours will overwrite // them @@ -926,7 +926,7 @@ function attachStatic( ctor, members, base, inheriting ) // TODO: This check is simple, but quick. It may be worth // setting a flag on the class during definition to specify if // it's extending from a non-class base. - return ( base.__cid && base.$ || exports.ClassBase.$ ).apply( + return ( base['__cid'] && base.$ || exports.ClassBase.$ ).apply( context, arguments ); } @@ -979,8 +979,8 @@ function attachStatic( ctor, members, base, inheriting ) */ function createMeta( func, cparent ) { - var id = func.__cid, - parent_meta = ( ( cparent.__cid ) + var id = func['__cid'], + parent_meta = ( ( cparent['__cid'] ) ? exports.getMeta( cparent ) : undefined ); @@ -1099,7 +1099,7 @@ function attachInstanceOf( instance ) */ exports.getMethodInstance = function( inst, cid ) { - var iid = inst.__iid, + var iid = inst['__iid'], data = inst.___$$vis$$; return ( iid && data ) diff --git a/lib/MemberBuilderValidator.js b/lib/MemberBuilderValidator.js index bee57e4..1fb03e2 100644 --- a/lib/MemberBuilderValidator.js +++ b/lib/MemberBuilderValidator.js @@ -163,7 +163,7 @@ exports.prototype.validateMethod = function( ); } } - else if ( keywords.override ) + else if ( keywords[ 'override' ] ) { // using the override keyword without a super method may indicate a bug, // but it shouldn't stop the class definition (it doesn't adversely diff --git a/lib/util.js b/lib/util.js index 281f269..d41f227 100644 --- a/lib/util.js +++ b/lib/util.js @@ -366,9 +366,11 @@ exports.createAbstractMethod = function() throw new Error( "Cannot call abstract method" ); }; - exports.defineSecureProp( method, 'abstractFlag', true ); - exports.defineSecureProp( method, 'definition', definition ); - exports.defineSecureProp( method, '__length', arguments.length ); + // TODO: should we protect these from modification (is it worth the + // overhead)? + method.abstractFlag = true; + method.definition = definition; + method.__length = arguments.length; return method; }; diff --git a/test/Interface/ExtendTest.js b/test/Interface/ExtendTest.js index 14b6fee..a868d7f 100644 --- a/test/Interface/ExtendTest.js +++ b/test/Interface/ExtendTest.js @@ -34,7 +34,9 @@ common.testCase( caseSetUp: function() { // There's a couple ways to create interfaces. Test 'em both. - this.baseTypes = [ + // (Note: we must quote this so that the name is not minified, since it + // is referenced within a string) + this['baseTypes'] = [ Interface.extend( { method: [], @@ -46,10 +48,10 @@ common.testCase( ]; // non-object values to assert failures upon - this.invalidExtend = [ 'moo', 5, false, undefined ]; + this['invalidExtend'] = [ 'moo', 5, false, undefined ]; // bad access modifiers (cannot be used in interfaces) - this.badAm = [ 'protected', 'private' ]; + this['badAm'] = [ 'protected', 'private' ]; }, diff --git a/test/MemberBuilderValidator/MethodTest.js b/test/MemberBuilderValidator/MethodTest.js index e78bd43..076cc91 100644 --- a/test/MemberBuilderValidator/MethodTest.js +++ b/test/MemberBuilderValidator/MethodTest.js @@ -32,8 +32,15 @@ require( 'common' ).testCase( this.quickKeywordMethodTest = function( keywords, identifier, prev ) { - shared.quickKeywordTest.call( this, - 'validateMethod', keywords, identifier, prev + shared.quickKeywordTest.call( _self, + // this awkward syntax (instead of simply passing a string) is + // to ensure the tests will still work after being compiled + function() + { + return _self.sut.validateMethod.apply( + _self.sut, arguments + ); + }, keywords, identifier, prev ); }; @@ -49,8 +56,8 @@ require( 'common' ).testCase( shared.quickVisChangeTest.call( _self, start, override, failtest, function( name, startobj, overrideobj ) { - startobj.virtual = true; - overrideobj.override = true; + startobj[ 'virtual' ] = true; + overrideobj[ 'override' ] = true; _self.sut.validateMethod( name, diff --git a/test/MemberBuilderValidator/PropertyTest.js b/test/MemberBuilderValidator/PropertyTest.js index dd80abe..baf0a89 100644 --- a/test/MemberBuilderValidator/PropertyTest.js +++ b/test/MemberBuilderValidator/PropertyTest.js @@ -37,8 +37,15 @@ require( 'common' ).testCase( this.quickKeywordPropTest = function( keywords, identifier, prev ) { - shared.quickKeywordTest.call( this, - 'validateProperty', keywords, identifier, prev + shared.quickKeywordTest.call( _self, + // this awkward syntax (instead of simply passing a string) is + // to ensure the tests will still work after being compiled + function() + { + return _self.sut.validateProperty.apply( + _self.sut, arguments + ); + }, keywords, identifier, prev ); }; diff --git a/test/MemberBuilderValidator/inc-common.js b/test/MemberBuilderValidator/inc-common.js index caf4514..b8280e3 100644 --- a/test/MemberBuilderValidator/inc-common.js +++ b/test/MemberBuilderValidator/inc-common.js @@ -111,7 +111,7 @@ exports.quickKeywordTest = function( type, keywords, identifier, prev ) var testfunc = function() { - _self.sut[ type ]( + type( name, function() {}, keyword_obj, prev_data, prev_obj ); }; diff --git a/test/inc-testcase.js b/test/inc-testcase.js index 4390828..a065899 100644 --- a/test/inc-testcase.js +++ b/test/inc-testcase.js @@ -41,7 +41,7 @@ var assert = require( 'assert' ), // wrap each of the assertions so that we can keep track of the number of times // that they were invoked -for ( f in assert ) +for ( var f in assert ) { var _assert_cur = assert[ f ]; @@ -105,7 +105,7 @@ module.exports = function( test_case ) delete test_case.setUp; // run each test in the case - for ( test in test_case ) + for ( var test in test_case ) { // xUnit-style setup if ( setUp ) @@ -282,7 +282,7 @@ function getMock( proto ) proto = Mock.prototype = new P() ; - for ( i in proto ) + for ( var i in proto ) { // only mock out methods if ( typeof proto[ i ] !== 'function' ) @@ -393,7 +393,26 @@ function outputTestFailures( failures ) */ var testPrint = ( ( typeof process === 'undefined' ) || ( typeof process.stdout === 'undefined' ) ) - ? function() {} + ? ( ( typeof console === 'undefined' ) + ? function() {} + : ( function() + { + var buffer = ''; + + return function( str ) + { + if ( str.search( '\n' ) !== -1 ) + { + console.log( buffer + str ); + buffer = ''; + + return; + } + + buffer += str; + } + } )() + ) : function( str ) { process.stdout.write( str ); diff --git a/test/test-class-abstract.js b/test/test-class-abstract.js index 9ba4085..6a1e73b 100644 --- a/test/test-class-abstract.js +++ b/test/test-class-abstract.js @@ -181,7 +181,7 @@ var ctor_called = false, // methods) var SubAbstractFoo = AbstractClass.extend( AbstractFoo, { - second: function() + 'second': function() { }, }); @@ -189,14 +189,14 @@ var SubAbstractFoo = AbstractClass.extend( AbstractFoo, // concrete var ConcreteFoo = Class.extend( AbstractFoo, { - method: function( one, two, three ) + 'method': function( one, two, three ) { // prevent Closure Compiler from optimizing the arguments away, causing // a definition failure return [ one, two, three ]; }, - second: function() + 'second': function() { }, }); diff --git a/test/test-class-constructor.js b/test/test-class-constructor.js index e036dab..b860480 100644 --- a/test/test-class-constructor.js +++ b/test/test-class-constructor.js @@ -64,9 +64,10 @@ assert.equal( "Constructor should be invoked once the class is instantiated" ); +// XXX: This may be encapsulated in the future assert.equal( - construct_context.__iid, - obj.__iid, + construct_context['__iid'], + obj['__iid'], "Constructor should be invoked within the context of the class instance" ); @@ -110,8 +111,8 @@ assert.equal( ); assert.equal( - construct_context.__iid, - subobj.__iid, + construct_context['__iid'], + subobj['__iid'], "Parent constructor is run in context of the subtype" ); @@ -133,8 +134,8 @@ assert.ok( ); assert.equal( - construct_context.__iid, - subobj2.__iid, + construct_context['__iid'], + subobj2['__iid'], "Self-invoking constructor is run in the context of the new object" ); diff --git a/test/test-class.js b/test/test-class.js index 982a4d8..3ae085e 100644 --- a/test/test-class.js +++ b/test/test-class.js @@ -143,13 +143,15 @@ assert.equal( +// XXX: Ultimately, this will be determined via reflection (at which time we +// need to determine if we want to let the compiler rename this) assert.ok( - ( Foo.__cid !== undefined ), + ( Foo['__cid'] !== undefined ), "Class id available via class" ); assert.ok( - ( Foo.prototype.__cid !== undefined ), + ( Foo.prototype['__cid'] !== undefined ), "Class id available via class prototype" ); diff --git a/test/test-combine-pre-es5.js b/test/test-combine-pre-es5.js index cc52a56..99a587e 100644 --- a/test/test-combine-pre-es5.js +++ b/test/test-combine-pre-es5.js @@ -34,6 +34,7 @@ var common = require( './common' ), // sandbox in which combined script will be run sandbox = vm.createContext( { // stub document.write() so we don't blow up + window: {}, document: { write: function() {} }, runTests: null, } ); @@ -72,5 +73,5 @@ data = "delete Object.defineProperty;" + vm.runInNewContext( data, sandbox ); // cross your fingers -sandbox.easejs.runTests(); +sandbox.window.easejs.runTests(); diff --git a/test/test-combine.js b/test/test-combine.js index e07cd2e..f980692 100644 --- a/test/test-combine.js +++ b/test/test-combine.js @@ -29,6 +29,7 @@ var common = require( './common' ), // sandbox in which combined script will be run sandbox = { // stub document.write() so we don't blow up + window: {}, document: { write: function() {} }, }; @@ -74,7 +75,7 @@ while ( i-- ) ); assert.ok( - ( sandbox.easejs !== undefined ), + ( sandbox.window.easejs !== undefined ), "'easejs' namespace is defined within combined file" ); @@ -86,7 +87,7 @@ while ( i-- ) ] .forEach( function( item ) { assert.ok( - sandbox.easejs[ item ], + sandbox.window.easejs[ item ], "Combined file exports exposes " + item ); } ); @@ -95,12 +96,12 @@ while ( i-- ) if ( file.match( /ease-full/ ) ) { assert.ok( - ( typeof sandbox.easejs.runTests === 'function' ), + ( typeof sandbox.window.easejs.runTests === 'function' ), "Full ease.js file contains test runner" ); // cross your fingers - sandbox.easejs.runTests(); + sandbox.window.easejs.runTests(); } } diff --git a/tools/combine b/tools/combine index 6fc726d..2c7b4a3 100755 --- a/tools/combine +++ b/tools/combine @@ -152,7 +152,7 @@ if [ "$INC_TEST" ]; then cat "$TPL_TEST_PATH" | grep -v '^#' | $RMTRAIL echo "/** TEST CASES **/" - echo "ns_exports.runTests = function()" + echo "ns_exports['runTests'] = function()" echo "{" for testcase in $TEST_INC $TEST_CASES; do diff --git a/tools/combine-test.tpl b/tools/combine-test.tpl index 1db427f..dfaf752 100644 --- a/tools/combine-test.tpl +++ b/tools/combine-test.tpl @@ -18,7 +18,7 @@ # along with this program. If not, see . # # -module.common = module['test/common'] = { exports: { +module['common'] = module['test/common'] = { exports: { require: function ( id ) { return require( id ); @@ -41,8 +41,10 @@ function failAssertion( err ) * Bare-bones implementation of node.js assert module * * This contains only the used assertions + * + * This is an export; as such, it is quoted to prevent minification. */ -module.assert = { exports: { +module['assert'] = { exports: { equal: function ( val, cmp, err ) { if ( val != cmp ) diff --git a/tools/combine.tpl b/tools/combine.tpl index 67496f8..761a1fd 100644 --- a/tools/combine.tpl +++ b/tools/combine.tpl @@ -11,9 +11,11 @@ * * var util = easejs.Class; * + * Note: Quoted for Closure Compiler export + * * @type {Object} */ -var easejs = {}; +window['easejs'] = {}; ( function( ns_exports ) { @@ -57,9 +59,9 @@ var easejs = {}; /**{CONTENT}**/ // the following should match the exports of /index.js - ns_exports.Class = module['class'].exports; - ns_exports.AbstractClass = module['class_abstract'].exports; - ns_exports.FinalClass = module['class_final'].exports; - ns_exports.Interface = module['interface'].exports; -} )( easejs ); + ns_exports['Class'] = module['class'].exports; + ns_exports['AbstractClass'] = module['class_abstract'].exports; + ns_exports['FinalClass'] = module['class_final'].exports; + ns_exports['Interface'] = module['interface'].exports; +} )( window['easejs'] );