Injecting objects directly in ExtJS Components
Opened this issue · 29 comments
It would be appreciable to be able to inject objects in ExtJS Components without inheriting from the base class. It's very handful when defining a view and some child items need only an injected store to work.
I.E. I have a form panel that has a combo box with a remote store :
Ext.define('MyForm', {
extend : 'Ext.form.Panel',
items : [{
xtype : 'combo',
fieldLabel : '...',
displayField : '...',
valueField : '..',
editable : false,
inject : {
store : 'myStore'
}
}]
)
It's really straightforward to achieve this, it's only necessary to add to Deft.core.Component the following override for the constructor
Ext.define( 'Deft.core.Component',
override: 'Ext.Component'
alternateClassName: [ 'Deft.Component' ]
constructor : do () ->
if Ext.getVersion( 'extjs' ) and Ext.getVersion( 'core' ).isLessThan( '4.1.0' )
return ( config ) ->
if( config isnt null and not @$injected and config.inject? )
Deft.Injector.inject( config.inject, @, false )
@$injected = true
return @callOverridden( arguments )
else
return ( config ) ->
if( config isnt null and not @$injected and config.inject? )
Deft.Injector.inject( config.inject, @, false )
@$injected = true
return @callParent( arguments )
I'm not sure however if it's worth testing if the object has already been injected ( if( config isnt null and not @$injected and config.inject? )) as it would be possible to override an already injected object in the class (maybe useful in some use cases...)
It would be maybe interesting to override also the initConfig method to allow more ExtJS classes to use the injection mechanism, it would be also possible to do the following :
Ext.create('MyClass', { inject : { ... } })
Without the need to resolve manually the dependence and to assign it to the instance.
However the basic solution I proposed is already really handy for me as I don't need to extend a class each time I have to inject a different store
The inject directive was designed to define class dependencies, rather than instance dependencies. For example, the Deft JS class preprocessor for inject takes into account the complexity of proper behavior for subclasses of a class that has inject directives.
Typically, I would expect that the specific store needed by your form component would be known by its parent view and set traditionally, rather than injected.
Ext.define('MyForm', {
extend : 'Ext.form.Panel',
inject: [ 'myStore' ],
items: [
{
xtype : 'combo',
fieldLabel : '...',
displayField : '...',
valueField : '..',
editable : false,
store: this.myStore
}
]
});
In the event you really do need to perform a runtime lookup of a dependency provider, rather than defining it via an inject directive as part of the class definition, you can also call:
Deft.Injector.resolve( 'myStore' )
to resolve a value for a dependency anywhere you might need it.
I don't see it likely that we would change Deft JS to support runtime overrides of the inject directive.
You are right, I didn't consider that @ refers to the context of the object being created, thus the store is already injected...
This example should be added to the user guide, it would have saved me some time :)
However supporting the runtime overrides of the inject directive is not a real performance concern and can be (maybe) useful.
I probably won't add it to the wiki because in my opinion a balance must be struck between giving multiple examples and not overwhelming new users reading the docs. That said, something like this would be an obvious part of most example applications. Now that the wiki is fairly complete I hope to have time to churn out at least one example app to make available. Stay tuned. ;-)
Err, your solution doesn't work, sorry. I made a mistake testing...
I could solve by overriding initComponent but it's too much verbose for me :)
As it seemed obvious to me, this refers to the window object and not to the class instance.
How to solve this? Runtime overrides of the inject? :)
Use initComponent(). This is the whole reason that method exists. From the ExtJS docs:
The initComponent template method is an important initialization step for a Component. It is intended to be implemented by each subclass of Ext.Component to provide any needed constructor logic.
Also, correctly using initComponent adds a grand total of 3 lines of code. That's the opposite of verbose.
Ext.define("MyForm", {
extend: "Ext.form.Panel",
inject: ["myStore"],
initComponent: function() {
Ext.apply(this, {
items: [
{
xtype: "combo",
fieldLabel: "...",
displayField: "...",
valueField: "...",
editable: false,
store: this.myStore
}
]
});
return this.callParent(arguments);
}
});So, you are using Coffee because it's less verbose and you propose to me to use a more verbose solution (that i know... ? ) ;)
There are no drawbacks in injecting objects at runtime, I don't understand why it should not be possible to do...
One main drawback of what you're proposing is to encourage people to do things the wrong way. You're supposed to be using initComponent(). This is what initComponent() is for. As the docs state, Sencha intends for all subclasses of component to implement this method.
Hi,
I feel injecting objects at runtime defeats the purpose of the DI design
pattern which is to resolve dependencies at boot time. If the app boots,
then all dependencies resolve.
2013/1/31 asiragusa notifications@github.com
So, you are using Coffee because it's less verbose and you propose to me
to use a more verbose solution (that i know... ? ) ;)There are no drawbacks in injecting objects at runtime, I don't understand
why it should not be possible to do...—
Reply to this email directly or view it on GitHubhttps://github.com//issues/77#issuecomment-12951115.
The dependences are resolved at boot time but injected in a different way of what is actually possible with DeftJS. It's exactly equivalent by calling manually
Deft.Injector.resolve( 'myStore' )
The doc states any needed constructor logic. If I don't need to apply any logic to construct my component, it's only a waste of time declaring the initComponent method...
And, by the way, what I proposed it's an addon to the functionalities of DeftJS that integrates well in the system.
The standard way of injecting the objects by a class preprocessor works well and doesn't need to change.
As a side note, the registerPreprocessor could be better written this way that is slightly faster ;)
registerPreprocessor: do() ->
if Ext.getVersion( 'extjs' ) and Ext.getVersion( 'core' ).isLessThan( '4.1.0' )
# Ext JS 4.0
return ( name, fn, position, relativeTo ) ->
Ext.Class.registerPreprocessor(
name
( Class, data, callback ) ->
return fn.call( @, Class, data, data, callback )
).setDefaultPreprocessorPosition( name, position, relativeTo )
else
return ( name, fn, position, relativeTo ) ->
# Sencha Touch 2.0+, Ext JS 4.1+
Ext.Class.registerPreprocessor(
name
( Class, data, hooks, callback ) ->
return fn.call( @, Class, data, hooks, callback )
[ name ]
position
relativeTo
)Creating and adding child components is constructor logic.
I'm with John: I just don't see the need to introduce a framework-specific component creation syntax when the standard syntax works fine when your component is built correctly. It's one thing to do this for class definition, but doing it for instance creation is even more intrusive.
It's also worth mentioning that correctly using initComponent() brings other advantages beyond simply passing injected objects to child components. The primary one being that you can pass in a configuration object at creation time to either supplement or override the configuration that the component defines by default.
Basically, the debate here seems centered around resistance to using initComponent(), which, again, is what you're supposed to be using anyway.
Thanks for the correction, @brian428. I had quickly tweaked the code from the original issue submission without testing it, and forgot to refactor the items config into initialization logic in initComponent(). In my own apps, I always use the initComponent() method when defining my views.
For the moment, I'm going to re-open this issue.
My gut feeling is that the inject directive is a peer to require and extend and is part of defining a class, not configuring an instance. That said, if a change to make it optionally support runtime overrides makes it easier to port existing Ext JS applications, integrate more easily with Sencha Designer or Architect output, or makes the Deft JS framework more approachable to new users, it's worth considering. I agree that you're supposed to be using initComponent(), but this is another case (like the use of initConfig()) where Sencha doesn't consistently follow their own API's guidance.
Additionally, we'll need to determine if it is indeed possible to support this across all of our target framework versions (both Ext JS and Sencha Touch) in a way that is likely to be future-proof.
@asiragusa - your use of immediate function invocation (via do()->) to assign the platform specific method implementation at class creation time is clever. While the change to registerProcessor isn't likely to provide measurable performance improvement since its only ever called once per class preprocessor, similar changes to more commonly executed functions such as those in the Deft.core.Componentoverride could certainly benefit. The Sencha Class System provides an optional parameter on Ext.define() that allows you the opportunity to procedurally modify your class definition, so similar assignment could also potentially be done there. I'll give some thought to which approach is clearer and pursue some optimization around the platform specific methods throughout Deft JS.
John items is a standard config parameter, so at least this time ExtJS guys are following their API's guidance ;)
I know that it's a slight improvement and doesn't change a lot, but maybe defining a Deft's global parameter as
Deft.parentMethod = "callParent"
or
Deft.parentMethod = "callOverridden"
could slash really a lot of code changing
@callParent( arguments )
and
@callOverridden( arguments )
by
@[Deft.parentMethod]( arguments )
This time the solution is quite slower but will allow to be more DRY compliant ;)
The third parameter of Ext.define is called each time the class is instantiated, with do you are doing the test once, at compile time ;)
It would be possible to hook the constructor of Ext.Base checking if the first parameter is an object containing the key inject. This would apply to every ExtJS class, and could be future proof. The inject config parameter (as I know) is not used as a config parameter for ExtJS classes.
More likely this solution would break client applications that are using a custom defined inject parameter, but that would happen however, as it's highly probable that inject is also defined in the class definition.
P.S: Writing DeftJS in straight JS would not allow an easier integration by the ExtJS team in their master branch?
Sorry, I'm not sure I understand the context for your Deft.parentMethod suggestion.
Regarding the third parameter of Ext.define():
createdFn : Function
Optional callback to execute after the class is created, the execution scope of which (this) will be the newly created class itself.`
For example, try this:
Ext.define( 'MyClass', {
constructor: function () {
console.log( 'Instance created.' );
this.callParent( arguments );
}
},
function () {
console.log( 'Class created.' );
}
);
Ext.create( 'MyClass' );
Ext.create( 'MyClass' );
Ext.create( 'MyClass' );
and you'll see:
Class created.
Instance created.
Instance created.
Instance created.
Regarding the viability of processing inject in Ext.Base's constructor:
- Deft JS injects dependencies before a class instance's constructor is called.
- In complex class hierarchies, we merge the dependencies requested in the
injectdirectives throughout the class hierarchy and fulfill them once before we call the leaf class's constructor.
Performing that logic in Ext.Base's constructor means that injected values would not be available until after all of the constructors in a class hierarchy chain have executed this.callParent() to reach it. That also assumes that all of the classes will call this.callParent(), which is unfortunately not a safe assumption.
My fault, I misreaded the doc :/
What I propose is something like
Ext.define( 'Deft.core.Class',
...
parentMethod : do() ->
if Ext.getVersion( 'extjs' ) and Ext.getVersion( 'core' ).isLessThan( '4.1.0' )
return "callOverridden"
else
return "callParent"and thus
@callParent( arguments )
would be written as
@[Deft.Class.parentMethod]( arguments )
Regarding inject, the fact that the injection is done during the constructor and not before is fine because you are pushing a config parameter, so it is not supposed to be defined before the class creation.
The fact is that, as you say, I'm seeing that the constructor of Ext.Base is not always called.
In every case the check for inject in the config parameter should be done for every class' constructor, so maybe it's worth hooking every class (and not only the classes that have the inject key)
Intercepting the constructor of EVERY CLASS seems really questionable to me.
And again, just so I'm sure I understand where everyone is at on this: we're talking about doing all this just to avoid using initComponent()?
Yes, you add 2 function calls and an if for every time you create the instance of a class.
It may seem questionable till you think about the workload that your browser has to do to instantiate any of ExtJS' classes, I'm sure that you will notice the difference... Or not? ;)
InitComponent is nonetheless a good argument but John gave a better answer to your question.
John has the final say, so I'll leave it to him to think about. To me, introducing a framework-specific component descriptor syntax and adding overhead to the creation of every instance of every class, just to avoid having an initComponent method (which is Sencha's intended way of doing this) seems inadvisable. If there's some underlying advantage in doing this, I'm not seeing it.
Yes
To be clear, in case my last reply was terse and made me sound like a jerk, I'm not trying to be. I'm just really conservative when it comes to adding more coupling to DeftJS. I feel that one of the benefits of DeftJS is it's relative unobtrusiveness.
It's not a problem, we are discussing about a not so straightforward problem and everybody is giving his best arguments to find the best solution.
There are at least 3 possible solutions:
- Doing nothing and leaving everything as it is. Sad, but I have already my own fork and it's not a big concern for me.
- Enabling the injection via constructor parameters only for
Ext.Component. My solution works quite well because the constructor ofExt.Componentis called by all the classes that inherit from and (if needed) the component logic and child components are treated ininitConfig(). It seems to me quite unlikely that you will need an injected object before having called the parent controller in the constructor of your own component. - Enabling the injection for every class, before the constructor is called. The most part of ExtJS classes accept as the first argument of the constructor a config object, it's a quite well established design pattern in all the Framework. Eventually it would be possible to look for an object containing
injectacross all the constructor arguments.
Solution n.2 overrides a base component of ExtJS via an override. It fits perfectly the philosophy of ExtJS, as overriding the base classes is a key functionality of the Framework. The weight added is 2 function calls and an if for every component created, that is, compared to the overall weight of constructing an ExtJS' Component, ridiculous.
Solution n.3 overrides every constructor and adds (also) 2 function calls and a if (eventually a for). We could think that this solution is intrusive, and maybe it is a little bit, but finally it consists in establishing a proxy that intercepts the constructors looking for and argument given. It's generic and fits the Framework quite well. The overhead of this solution would be (maybe) slightly noticeable only in the case of instantiating thousands of small classes in a row, and I think that it's really a bad habit that really few developers would do.
Solved the problem of the overhead there is the coupling with ExtJS. The first solution fits perfectly, the second is as much intrusive as the rest, as you are overriding the constructor only in the case of the inject key (and others) in the object.
If you have other questions don't hesitate to ask and participate in a propositive way (as John does ;), it makes often interesting subjects, instead of boring flames.
When I say "intrusive", I don't mean the invisible things that DeftJS does under the hood like intercepting constructors, because the developer never knows that is even happening. I mean explicit coupling like introducing injection configuration into the component descriptors.
I quote John:
My gut feeling is that the inject directive is a peer to require and extend and is part of defining a class, not configuring an instance. That said, if a change to make it optionally support runtime overrides makes it easier to port existing Ext JS applications, integrate more easily with Sencha Designer or Architect output, or makes the Deft JS framework more approachable to new users, it's worth considering.
That said I'll give you the main use cases I'm thinking about for the new syntax, against the old way:
Ext.define('MyForm', {
extend : 'Ext.form.Panel',
items: [
{
xtype : 'combo',
inject : {
store : 'myStore'
}
}
]
});Or:
Ext.define("MyForm", {
extend: "Ext.form.Panel",
initComponent: function() {
Ext.apply(this, {
items: [
{
xtype: "combo",
inject : {
store : 'myStore'
}
}
]
});
return this.callParent(arguments);
}
});Against the old way:
Ext.define("MyForm", {
extend: "Ext.form.Panel",
inject: ["myStore"],
initComponent: function() {
Ext.apply(this, {
items: [
{
xtype: "combo",
store: this.myStore
}
]
});
return this.callParent(arguments);
}
});Or
Ext.define("MyForm", {
extend: "Ext.form.Panel",
initComponent: function() {
Ext.apply(this, {
items: [
{
xtype: "combo",
store: Deft.injector.resolve( 'myStore' )
}
]
});
return this.callParent(arguments);
}
});The syntax I propose seems to me more handy and useful for a better integration with ExtJS and the derivates (Architect etc...) against an insignificant overhead during the class instantiation.
It seems to me also more logic and natural using this syntax, it's the first thing at which I think when I want to inject an object in a child component.
The level of coupling of both solutions is exactly the same. The class is totally unaware of what happens under the hood, the developer can feel free using the same syntax he's using already for injecting objects in his classes.
Notice that the second and third code blocks you show there have exactly the same amount of code. The only difference (and this is what I'm talking about when I mention coupling) is that the second block uses a DeftJS-specific syntax to create the combobox. We already have coupling to DeftJS for CLASS DEFINITION (inject: and controller:). This would add coupling to DeftJS for INSTANCE CREATION as well.
A part the fact that it's not a drama, however in 3 you are injecting an object in a view that is potentially not concerned by what you are injecting into.
In my example have a combo box with a remote store that fetches the values from the server. The whole form doesn't need to have a reference to the combo's store (as it's not concerned by it). Why should I inject my object in the form to make things work? (This is why I also made example 4)
In any case the instance creation is already hooked for the classes that use injected objects and view controllers (as the injection is done during the instance creation), almost nothing changes in the way Deft JS works.
Sorry but this discussion begins tiring me... Does John have more constructive thoughts?
In that case, the last thing I'll say is that you seem to be mistaking the invisible constructor interception that DeftJS does as "coupling". This isn't coupling, because the developer has no knowledge that it is occurring, and it doesn't affect the code they have to write.
But this:
xtype: "combo",
inject : {
store : 'myStore"
}
the developer DOES have to have knowledge that it is occurring and it DOES affect the code they have to write. It changes the way you create an object from using the standard approach to using a DeftJS-specific approach.
Anyway, I'll let this sit while we see what John thinks.
Thanks, asiragusa. Have a good weekend.
And in 3 and 4 no? LOL
You too, maybe monday I'll have more pertinent answers ;)