deftjs / DeftJS

Extensions for Large-Scale Sencha Touch and Ext JS Applications
http://deftjs.org/
MIT License
285 stars 56 forks source link

In current version it's not possible to control an Ext.view.View #71

Closed asiragusa closed 11 years ago

asiragusa commented 11 years ago

In current version it's not possible to control an Ext.view.View. I have modified Controllable and ViewController to change the inheritance test from Ext.Container to Ext.Component as it should be.

Moreover I added a test in Controllable that raises an error in case of wrong inheritance.

brian428 commented 11 years ago

Just so we know: did you run the Jasmine tests for ExtJS and Touch after you made these changes? Thanks.

asiragusa commented 11 years ago

Nope, but I'll do for the next commits

asiragusa commented 11 years ago

By the way where are located these tests? I have done another bugfix

brian428 commented 11 years ago

The Jasmine runner is in the /spec folder. You can change which Sencha library is included to run the tests using Touch instead of ExtJS.

asiragusa commented 11 years ago

I run all the tests and updated the one that was failing (should throw an error if created and configured with a non-Ext.Container as the view)

However there are still three tests that are failing for ExtJS 4.0.7 (as in master branch)

asiragusa commented 11 years ago

This commit fixes a bug that affects DeftJS when used with Ext.Loader

In my app.js i have the following :

Ext.Loader.setPath({ 'Deft' : '/projectRoot/Deft', ... my libs ... });

and

Ext.require([ 'Deft.*', ... other requires ... ]);

I do $sencha app refresh to update the bootstrap.js with all the dependencies and I refresh my app in the browser.

ExtJS throws the following error :

Uncaught Error: The following classes are not declared even if their files have been loaded: 'Deft.Component'. Please check the source code of their corresponding files for possible typos: 'Deft/event/LiveEventBus.js

The error was due to the inlined Ext.define in LiveEventBus

I have externalized the class in a new file and moved the tests for the touch version

asiragusa commented 11 years ago

I also added late observers. In the case where the store is not a singleton, it's not possible to inject a reference in the controller, so in the observe config instead of observing the store directly we would do the following:

Ext.define('MyController, { extend: 'Deft.mvc.ViewController',

observe: {
    'view.store': {
        remove: "onRecordRemoved"
    }
},

})

But the view instance is assigned to the controller after the observer's creation.

One solution is to give the reference of the view to the controller constructor, but the Observable mixin of the view is not initialized at that moment.

Calling the controller constructor after the @callParent( arguments ) in Controllable works, but it breaks the test suite.

So I decided to add a new type of observers, that are initialized before the init of the controller.

Now we are able to do the following:

Ext.define('MyController, { extend: 'Deft.mvc.ViewController',

observeLate: {
    'view.store': {
        remove: "onRecordRemoved"
    }
}

})

Another possible solution is to try to observe the object at two different moments, in the controller constructor and before the init, but honestly for me editing these coffee script is the nightmare... IMHO with plain JS the code would be cleaner, easier to modify and faster...

With observeLate all the tests are passed, but I've not written a test suite for

brian428 commented 11 years ago

Two suggestions. First, I would separate the proposed changes to Observe out into a separate pull request, because this one is getting pretty broad. And second, I would probably rather see an additional "late: true" config option when defining an Observer over having a separate observeLate property. Duplicate properties that do virtually the same thing is opening a very slippery slope, IMO.

John and I both use CoffeeScript for everything we do, so that won't be changing. It's actually far superior to straight JavaScript. ;-) That said, if you're more comfortable in JavaScript, you can always code in JS and run your code through http://js2coffee.org/. You'd still have some formatting tweaks to make (JS2Coffee uses implicit parentheses, for example), but it might make it easier if you really prefer to work in JS.

asiragusa commented 11 years ago

Yep I agree with the option late : true, it was my preferred choice but.. how to say... coffee was killing me :)

I'll give a try to js2coffee, I hope it will save some time.

brian428 commented 11 years ago

It's really very similar to JavaScript, except without all the clutter of nested braces and semicolons, and with a bunch of great syntactic sugar to simply common things. Once you get used to it (which took me less than a day), you'll probably never want to write JavaScript again. ;-)

asiragusa commented 11 years ago

It has some good shapes, the tests are really good looking, but its downside is that you tend to write longer and more nested methods because it's more readable. However parenthesis make the code easier to edit, to spot indentation errors and to see where a statement ends.

I prefer to write my methods in "java style": you have sanity tests at the beginning and you return immediately in case of error.

Nesting is always overkill for your application, it's more difficult to be DRY and to make evolutions.

Look at the mergeObserve method in Observer, modifying its structure is really hard because of its over-indentation, separating the code in smaller chunks of code would make the method more readable and easier to extend.

Do you see my point? ;)

brian428 commented 11 years ago

CS really doesn't cause more nesting (at least for me). mergeObserve is a somewhat unique case for two reasons: first, it's static, and second, it's optimized for speed. Remember that all this has to happen before the classes themselves are registered with the ClassManager, and it has to happen up-front for every ViewController. So speed here trumps just about any other concern. You'll notice that the rest of the class uses smaller methods, because that logic happens lazily (only once a given VC is instantiated).

asiragusa commented 11 years ago

Honestly I think that this is a really cool project that fixes some huge problems ExtJS has. Your are adopting the correct design patterns to solve these problems and it doesn't impact the structure of the Framework, so switching to DeftJS is really straightforward.

But, there is also a problem IMHO :)

Coffee, even if you learn to handle it, makes you waste more time in developing against JavaScript.

To do an example, with eclipse you have code completion over ExtJS and real time error checking, things that you loose with CS...

Having an IDE that marks directly the error in red or yellow it's worth nothing! (Specially when you have typos or errors dues to copy paste, and you know that it's really frequent...)

Moreover CS is based on tab spaces, and they are not handled everywhere in the same way. Eclipse (my f*cking editor in this case :) does a mess with this by default...

Doing the light bugfix I did was a really pain a cause of CS and made me waste a lot of time, not because I didn't know how to solve the problem, but a cause of the syntax.

Moreover I don't understand why to write some code and to debug it in another language (and finally the output is more verbose than it should be and quite badly indented)

I'm developing a huge application in ExtJS with multiple views of the same type (many windows that contain sub applications), so this project is gold for me :) But how can I contribute if I have to waste a lot of time a cause of CS? (And I think that other contributors could be discouraged a cause of this and it's a shame...)

brian428 commented 11 years ago

We're getting pretty off topic for a pull request. If you want to continue the discussion about CoffeeScript on the mailing list, feel free to post there. Sound good? Thanks.

asiragusa commented 11 years ago

Yeah you are right, it's OT. But it would be OT in the ML also, I see that all of the main contributors (so you also :) are working or worked in the same enterprise and all of you are strong believers in Coffee => no clue to change attitudes...

I will consider in the future if participating actively or passively (opening new issues) at this project.

P.S: To be really OT, It's better Italian coffee ;)

Ciao!

brian428 commented 11 years ago

I don't think it would be off topic on the mailing list at all. But I won't pressure you. ;-)

brian428 commented 11 years ago

Added this behavior to Deft 0.8.0.