-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Injecting objects directly in ExtJS Components #77
Comments
The 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.
In the event you really do need to perform a runtime lookup of a dependency provider, rather than defining it via an
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 |
You are right, I didn't consider that This example should be added to the user guide, it would have saved me some time :) However supporting the runtime overrides of the |
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 |
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:
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 2013/1/31 asiragusa [email protected]
|
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
The doc states |
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 For the moment, I'm going to re-open this issue. My gut feeling is that the 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 |
John I know that it's a slight improvement and doesn't change a lot, but maybe defining a Deft's global parameter as
could slash really a lot of code changing
and
by
This time the solution is quite slower but will allow to be more DRY compliant ;) The third parameter of It would be possible to hook the constructor of 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 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 Regarding the third parameter of
For example, try this:
and you'll see:
Regarding the viability of processing
Performing that logic in |
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
would be written as
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 |
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:
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:
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:
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 ;) |
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 text was updated successfully, but these errors were encountered: