{"action":"created","issue":{"url":"https://api.github.com/repos/handsontable/handsontable/issues/3744","repository_url":"https://api.github.com/repos/handsontable/handsontable","labels_url":"https://api.github.com/repos/handsontable/handsontable/issues/3744/labels{/name}","comments_url":"https://api.github.com/repos/handsontable/handsontable/issues/3744/comments","events_url":"https://api.github.com/repos/handsontable/handsontable/issues/3744/events","html_url":"https://github.com/handsontable/handsontable/issues/3744","id":175042501,"number":3744,"title":"Schema check relies on key order","user":{"login":"rowanhill","id":2607287,"avatar_url":"https://avatars.githubusercontent.com/u/2607287?v=3","gravatar_id":"","url":"https://api.github.com/users/rowanhill","html_url":"https://github.com/rowanhill","followers_url":"https://api.github.com/users/rowanhill/followers","following_url":"https://api.github.com/users/rowanhill/following{/other_user}","gists_url":"https://api.github.com/users/rowanhill/gists{/gist_id}","starred_url":"https://api.github.com/users/rowanhill/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rowanhill/subscriptions","organizations_url":"https://api.github.com/users/rowanhill/orgs","repos_url":"https://api.github.com/users/rowanhill/repos","events_url":"https://api.github.com/users/rowanhill/events{/privacy}","received_events_url":"https://api.github.com/users/rowanhill/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/handsontable/handsontable/labels/Assistance%20needed","name":"Assistance needed","color":"0052cc"},{"url":"https://api.github.com/repos/handsontable/handsontable/labels/Core:%20handsontable","name":"Core: handsontable","color":"009800"},{"url":"https://api.github.com/repos/handsontable/handsontable/labels/Question","name":"Question","color":"0052cc"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-09-05T10:29:10Z","updated_at":"2016-09-05T13:48:17Z","closed_at":null,"body":"Hi there,\r\n\r\nWe've been using handsontable with great success so far, so first of all, thanks!\r\n\r\nI've recently come across a problem which, I think, is arguably a bug. I have a table where all cells hold values of the same type of object (which we call AllocationCell), where some properties may be as-yet undefined. The issue is that if I autofill from one cell to another (say, dragging from A1 to B1), and then try to autofill a second time from the newly filled cell to a third cell (say, dragging B1 to B2) then the second autofill silently fails.\r\n\r\nDigging into this, it's because populateFromArray has [checked the schemas](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/core.js#L508) of the values and determined that they're different.\r\n\r\nMy understanding of the sequence of events is:\r\n 1.  I drag from A1 to B1\r\n 2. populateFromArray tests the schemas, and finds them identical\r\n 3. [A deep clone is taken](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/core.js#L509) of A1, which ultimately becomes the value of B1\r\n 4. Taking that deep clone [uses JSON.stringify and JSON.parse](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/helpers/object.js#L102-L108), which means the cloned object doesn't have the same prototype as the original. If any properties were undefined, those properties are now completely absent from the clone.\r\n 5. I drag from B2 to B2\r\n 6. populateFromArray [creates \"duck-typing\" schemas](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/helpers/object.js#L10-L36) by [iterating over the keys](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/helpers/object.js#L249-L259) of the objects, and setting null values\r\n 7. populateFromArray compares the schemas [by comparing their JSON strings](https://github.com/handsontable/handsontable/blob/b59c52037fe0c319a3f6d21ad2542075d1ab74cb/src/helpers/object.js#L193-L195).\r\n 8. Because B1 is a clone (with no prototype, and missing properties) it has a different schema to B2, and the autofill fails.\r\n\r\nIn practice, my case is slightly worse even than this, because my afterChange handler sets the missing properties on the cloned object, so the two schemas do actually have the same keys, but because the clone is missing the prototype stringified JSON lists them in different orders.\r\n\r\nI can reproduce the failure in [this JSFiddle](https://jsfiddle.net/wLcz48j7/5/) (on Chrome 52 & 53, at least - I haven't tried other browsers).\r\n\r\nI think there's arguably a bug here, because handsontable is relying on the the ordering of keys in the results of JSON.stringify, but I believe [that's never guaranteed](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Description):\r\n\r\n> Properties of non-array objects are not guaranteed to be stringified in any particular order. Do not rely on ordering of properties within the same object within the stringification.\r\n\r\nThat said, it's also just fluke that my application happens to set all the properties on the clone; it could quite conceivably simply leave some properties missing, so I think even using a JSON stringify with stable ordering (e.g. [json-stable-stringify](https://github.com/substack/json-stable-stringify) would be insufficient.\r\n\r\nI can think of a few potential solutions:\r\n 1. Mandate that all cell values always define all properties they use (e.g. AllocationCell could set everything to null, rather than leaving them undefined), and either use json-stable-strinigify (or similar) or carry on trusting that modern browsers will do something sensible here. That seems a bit of a shame, because a) it's a surprising requirement that won't be easy to communicate to devs, b) said devs might genuinely want some values to be undefined, and it's a bit unpleasant to ask them to not do so.\r\n 2. Implement full-on key order independent deep schema checking - handsontable wouldn't need to use duckSchema anymore, or use JSON.stringify to test equality. It would be quite a chunk of code to write, though, and probably the sort of thing that'll be a bit buggy initially.\r\n 3. Leave the current default behaviour (seeing as it seems to work for most people), but allow a \"schema comparison\" function to be provided when setting up handsontable. That would allow devs to write something tailored to their specific situation, but adds to the complexity of handsontable, and makes it a bit less \"out-of-the-box ready\".\r\n\r\nSlightly tangentially to the above, I also wonder if there should be (or already is?) some way of getting feedback when handsontable decides to cancel a user action. It would have been nice to have been told that I couldn't do the second autofill because the schemas of the values didn't match, rather than have to read through lots of code.\r\n\r\nSo, can I end by asking a few questions of the maintainers:\r\n- Do you agree this is a bug and is worth addressing?\r\n- Do you have any thoughts on possible solutions (either the ones I've listed above, or anything you can think of)?\r\n- Is there / should there be a way of giving devs feedback when cancelling user actions?\r\n\r\nThanks!\r\nRowan"},"comment":{"url":"https://api.github.com/repos/handsontable/handsontable/issues/comments/244751155","html_url":"https://github.com/handsontable/handsontable/issues/3744#issuecomment-244751155","issue_url":"https://api.github.com/repos/handsontable/handsontable/issues/3744","id":244751155,"user":{"login":"AMBudnik","id":10757813,"avatar_url":"https://avatars.githubusercontent.com/u/10757813?v=3","gravatar_id":"","url":"https://api.github.com/users/AMBudnik","html_url":"https://github.com/AMBudnik","followers_url":"https://api.github.com/users/AMBudnik/followers","following_url":"https://api.github.com/users/AMBudnik/following{/other_user}","gists_url":"https://api.github.com/users/AMBudnik/gists{/gist_id}","starred_url":"https://api.github.com/users/AMBudnik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/AMBudnik/subscriptions","organizations_url":"https://api.github.com/users/AMBudnik/orgs","repos_url":"https://api.github.com/users/AMBudnik/repos","events_url":"https://api.github.com/users/AMBudnik/events{/privacy}","received_events_url":"https://api.github.com/users/AMBudnik/received_events","type":"User","site_admin":false},"created_at":"2016-09-05T13:48:17Z","updated_at":"2016-09-05T13:48:17Z","body":"That's a good tactic as you never know what can come in handy and every tip can be crucial. \r\nps. I've already send the issue link to one of our devs.\r\nI will keep you updated. Thanks again!"}}
{"actor":{"display_login":"AMBudnik"}}