Skip to content
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

No type coersion on dataView.getItemById(row_id); #670

Closed
syonfox opened this issue Mar 31, 2022 · 2 comments
Closed

No type coersion on dataView.getItemById(row_id); #670

syonfox opened this issue Mar 31, 2022 · 2 comments

Comments

@syonfox
Copy link

syonfox commented Mar 31, 2022

To this is easy enough to work around but SHould probably be changed.

data =  [{id: 1, foo:'bar1'},{id: 2, foo:'bar2'}]

let row = grid.dataView.getItemById(1); //works
let row = grid.dataView.getItemById('1'); //doesn't work

In my system some table have string uuid's and some have just integers but i want to support both.

It seems that data view puts things in a map which ends up using 'has' to check for an id

SlickGrid/slick.core.js

Lines 655 to 663 in 1db791d

/***
* Gets a value indicating whether the given key is present in the map.
* @method has
* @param key The key of the item in the map.
* @return {Boolean}
*/
this.has = function(key) {
return key in data;
};

This is confusing since the in operator seems to PROPERLY accept strings or numbers for looking up keys.

My workaround is kind of hacky but seems to work.

            let row = grid.dataView.getItemById(row_id);
            if(!row) row = grid.dataView.getItemById(parseInt(row_id));
            if(!row) console.error("Unable to find row in data view with that ID you sure its right? id:", row_id);

Does anyone have thoughts on why this might happen. or if there is a better fix. Thanks

@ghiscoding
Copy link
Collaborator

This is normal and no I don't consider it to be hacky and yes it's because it got switched to Map with PR #572 to optimize bulk operations.

Technically speaking it's the developer's responsibility to know and use the correct Type (it could be either a number or string and SlickGrid doesn't know which Type you're using in your dataset) and also since SlickGrid is a JavaScript lib (not a TypeScript lib) well there's no way to properly advise you to use the correct Type when querying the lib and most developers will only ever have 1 Type not 2 Types like your use case (which is very rare).

Considering the performance gain that we've got with PR #572, we will stick with Map, so again it's your responsibility to query the data in the correct Type, I don't see any problem with the lib.

@syonfox
Copy link
Author

syonfox commented Apr 11, 2022

Sounds good thanks, You are right that it is easy enough to detect the type and cast before the function call. And I cant see a use case where there are 2 different types in the id field. Worst case one could search the data the js way data.find(x=>x.id = row_id) but that probably doesn't help performance.

The real question is probably where is my code changing the int to a string or vise versa

@syonfox syonfox closed this as completed Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants