Changing datatype default value

[bouks]

Are you ok to change the default "datatype" value ?

https://github.com/openpsa/grid.js/pull/30#issuecomment-69546996

[meh-uk]

I am yes.

[flack]

Well, the PR was merged, so technically, the change already happened anyways :-)

For me, it's ok. especially since existing client code doesn't need to be changed. It would be interesting to get @OlegKi's opinion, because he changed the default, too (to json). So the question is if he will merge your change into his repo as well. If not, we will have to manually merge commits touching these lines from here on in

[bouks]

Seems he changed to 'local' :)

https://github.com/OlegKi/jqGrid/commit/aeb619246af73faa7a50678c7458a43df82554ff

[meh-uk]

That sounds the most sensible.

[OlegKi]

I agree that the changing of the default value of datatype is sensible and it can break the code till the people read the readme, compatibility remark and so on and till the code will include explicitly the option datatype: "xml".

To be clear: The problem exists only for grids which loads XML data from the server and which don't explicitly use any datatype option. In my experience the number of usages of datatype: "xml" is dramatically reduced last time. I write here only about the question on the stackoverflow which I see. jQuery don't have any good XML parser. XML data which uses XML Schema can't be good processed using jQuery (at least I don't know any solution which works browser independent). I personally like XML, but I use it always with the corresponding XSD. Moreover one can't load the most existing XML data from existing services because of the usage of XML Schema. Another problem is the performance: XML data will be loaded and parsed more slowly as JSON data.

The last time more and more will be used datatype: "local" or datatype: "json" together with loadonce: true. One loads the data from the server in some way and then one creates the grid with the data using input data parameter. The performance of the current versions of web browsers allows to process the data with about 1000 rows really very quickly (especially in case of usage local paging and not so large number of columns). The user can see results of filtering and sorting practically immediately.

I suppose that datatype: "local" will be used more frequent in the future. Moreover it's the option which try new users of jqGrid as the first. It's not an accident, that the first example placed on the new documentation of openpsa/grid.js was an example with datatype: "local". Moreover it's the only option where jqGrid don't make any Ajax request to the server if the user uses minimal number of options. Just try the demo which use the simplest code $("#list").jqGrid({colModel: [{ name: "colName" }]});. You will see that the demo makes HTTP GET request to http://www.ok-soft-gmbh.com/jqGrid/small.htm?_search=false&nd=1421084502888&rows=20&page=1&sidx=&sord=asc whith Accept: application/xml, text/xml in one from the HTTP headers. I find allays such default behavior as very bewildering.

The newcomer are wonder why jqGrid makes some Ajax requests to the server in case of usage the simplest options. In any way the usage of "xml" as the default (and so preferred) option seem me not the best choice. All described above was the reason why I do decided to change default value to datatype in spite of possible incompatibility problems.

[flack]

@OlegKi I think we can all agree that "local" makes a better default than "xml". And for people reyling on "xml" implicitly, we can add a big fat note in the release notes. But I still think "auto" would make a good default, too, even if it probably does not work in each and every case. For example, then your simplest local grid would look like this:

$('#list').jqGrid({
    colModel: [ name: "colName"],
    data: some_data
});

and the simplest remote grid would look like this:

$('#list').jqGrid({
    colModel: [ name: "colName"],
    url: "/backend.php"
});

[flack]

P.S.: And your original code

$('#list').jqGrid({
    colModel: [ name: "colName"]
});

would still work (almost) the same way as in jqGrid 4.7.0, thus providing backward compatibility

[OlegKi]

I included the example with

    $('#list').jqGrid({
        colModel: [ name: "colName"]
    });

only to show than the old version of jqGrid do some things which I would like to change, so to break some backward compatibility.

@flack : What do you thing about the usage of

$('#list').jqGrid({
    colModel: [ name: "colName"],
    url: "/backend.php",
    loadonce: true
});

How you suggest to detect datatype? I'm sure that one can implement good auto-detection of datatype, but I don't think that one will be able do this already in the first new release.

[bouks]

@OlegKi

With this last example and the actual code the result will be well detected even if there is a mimetype (Content-Type) specified in the http header or not.

[flack]

@OlegKi I'm not sure I understand the problem with loadonce and autodetection. loadonce: true means that the datatype is change to "local" after the initial request is made, right? If so, we could simply do the same after autodetection, e.g.

var autodetected_type = run_autodetection_logic();
if (    grid.loadonce === true
    && (autodetected_type === 'xml' || autodetected_type === 'json'))
{
    grid.datatype = 'local';
}
else
{
    grid.datatype = autodetected_type;
}

[flack]

@OlegKi: We don't have to keep the old default behaviour, the above was just to show that it is possible to do so. If we want to change the default behaviour, I would suggest changing the default value for url to null, and then skip the AJAX request in this case. I've created a separate ticket to better track that change: #46

[bouks]

Note that with loadonce: true the datatype is changed to local even if no initial request has been made.

I tried to pull request a demo but i don't know how to publish, and in this demo, if you loadonce: true it will never work until you change the datatype other than 'local' (only for this example by populating with button, if url is in grid option, no problem).

I think there is confusion in datatype since the early begining. "local" is not a datatype, it's a state. I think we must redefine "local" out of datatype.

[bouks]

Excuse me but I really don't agree to put datatype in local by default.

• It breaks the idea of autodetection
• I think most people get datas by calling url and/or use of xml|json

From release note : "datatype defaults to "local": If you use XML data, you should add datatype: "xml" to override the default."

[flack]

actually I think the release notes are already outdated. Currently, there doesn't seem to be any default. This seems to come from here: https://github.com/OlegKi/jqGrid/commit/57128368c989ddf691de663fd5f4756d82a29472

[flack]

correction: It comes from here: https://github.com/openpsa/grid.js/commit/beee9e6c2c04943dc4235750d94cbd46329e1147. The other commit we haven't merged (at least not yet)

[bouks]

Ok.

I think autodetection is better than :

https://github.com/openpsa/grid.js/blob/master/js/grid.base.js#L1203

Also

https://github.com/openpsa/grid.js/blob/master/js/grid.base.js#L1206

and

https://github.com/openpsa/grid.js/blob/master/js/grid.base.js#L2890

It's confusing.

[flack]

I like that the current version can detect local automatically, but the part about "xml" and especially jsonReader is confusing. Couldn't we hook in your remote autodetection logic here:

https://github.com/openpsa/grid.js/blob/master/js/grid.base.js#L1206

The we could have both cases, local and remote. BTW this is covered in #66 and #46 and #38. We should maybe try to centralize the discussion in one of those :)

[meh-uk]

Can we close this?

[bouks]

See discussion here: https://github.com/openpsa/jsgrid/issues/46