Proxy.js has Uses where it needs Requires

[mark0978]

Because of this line in the constructor:

        if (this.model !== undefined && !(this.model instanceof Ext.data.Model)) {
            this.setModel(this.model);
        }

Ext.data.Model should move from the uses to the requires array, like so:

Ext.define('Ext.data.proxy.Proxy', {
    alias: 'proxy.proxy',
    alternateClassName: ['Ext.data.DataProxy', 'Ext.data.Proxy'],
    requires: [
        'Ext.data.reader.Json',
        'Ext.data.writer.Json',
        'Ext.data.Model'
    ],
    uses: [
        'Ext.data.Batch', 
        'Ext.data.Operation', 
        //'Ext.data.Model'
    ],

[mitchellsimoens]

Can I get a test case where this is a problem?

Ext.data.Store has Ext.data.Model in the requires so the Ext.data.Model will be required before the proxy is created.

[mark0978]

A Test case is not easily chopped out of my app. Static analysis should show you all you need to know. You can't uses Ext.data.Model in the constructor unless is is defined

Which part of this don't you understand?

if (this.model !== undefined && !(this.model instanceof Ext.data.Model))

The && means it will be evaluated if you are going to successfully create your Proxy, and you can't use instanceof on an undefined type.

[mitchellsimoens]

But where will a proxy be used without a Store or a Model? It could actually be removed from the uses and have zero effect except save a little framework size.

[mark0978]

It can't be used without the Model which is why the requires is necessary. You pass the Model NAME (as a string) into the Proxy and you error on the isinstanceof 'Ext.data.Model' because Ext.data.Model does not exist because it wasn't required.

Part of this probably plays out because of this issue

http://www.sencha.com/forum/showthre....js-works.-Why

Which references another bug.

[LesJ]

It could actually be removed from the uses and have zero effect except save a little framework size.

This would be a mistake as the code base would become harder to understand. Looking at Ext.define, I'd like to know class dependencies even if they are guaranteed to be already loaded.

[mitchellsimoens]

Ext.define('MyModel', {
    extend : 'Ext.data.Model',

    fields : [...],

    proxy : {
        type : 'ajax',
        ....
    }
});

Ext.define('MyStore', {
    extend : 'Ext.data.Store',

    model : 'SomeModel',

    proxy : {
        type : 'ajax',
        ....
    }
});

There really is no reason to ever Ext.create('Ext.data.proxy.Ajax', {...})

And I said we *could* remove it and it would have no affect, not saying we are going to.

[mark0978]

Here is what I'm doing:

Ext.define('IHawk.viewer.data.BatchProxy', {
    extend: 'Ext.data.proxy.Ajax',
    model: 'Batch',
...

[LesJ]

What's the problem to move the dependency from uses to requires? This would make the code more readable even if it doesn't fix anything.

[skirtle]

In my opinion the requires for a class needs to be correct. Currently it is only used for dynamic loading but who knows what other uses the requires property will have in future, for example in build utilities.

Given the current code I would say that Ext.data.Model needs to be in the requires for a proxy. The argument that proxies are only ever used with stores/models doesn't work for me.

One use case (off the top of my head) would be a singleton proxy instance that is shared between many stores. This may be defined before the store/model is loaded and currently I think that would fail.

I would actually suggest that a better solution to this is to remove the instanceof check and use isModel instead. Something like this maybe:

if (this.model && !this.model.isModel)) {

There's probably a good reason why the original code explicitly checked for undefined rather than truthyness, but you get the idea.

This removes the dependency on Ext.data.Model. However, there's still a dependency on Ext.ModelManager that I think should be explicitly included in the requires.