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

Breaking change in function behavior getItemById ( Dataview ) #574

Closed
bzhjack opened this issue Feb 10, 2021 · 5 comments
Closed

Breaking change in function behavior getItemById ( Dataview ) #574

bzhjack opened this issue Feb 10, 2021 · 5 comments

Comments

@bzhjack
Copy link

bzhjack commented Feb 10, 2021

In the version 2.4.32

const Item = this.dataView.getItemById(id); 

with id = '99' or 99 it works.

In version 2.4.33 with id = '99' getItemById does not work anymore

// work
function getItemById(id) {
      return items[idxById[id]];
    }
//  does not work

function getItemById(id) {
return items[idxById.get(id)];
 }
@ghiscoding
Copy link
Collaborator

ghiscoding commented Feb 10, 2021

The latest @Danielku15 Pull Request #572 added optimization for bulk operations and changed the code to use Map & Set, if we look at the description of Map on MDN, it says:

Key equality is based on the sameValueZero algorithm.

I don't think we'll do any code change to fix this, it makes sense that the Type you pass in your SETTER must be the same Type as how you retrieve it with your GETTER. In your use case, you can just add a + in front of your code and you'll be good to go.
const Item = this.dataView.getItemById(+id);

The move to Map is for efficiency, I don't see us rolling back on this, you need to adjust your code.

@bzhjack
Copy link
Author

bzhjack commented Feb 10, 2021

yes I see, but I use this function generically in an Angular component without knowing in advance the nature of the data source and therefore that of the id.

@ghiscoding
Copy link
Collaborator

well technically speaking Angular uses TypeScript which generally enforces strict equality, it's generally bad practice not follow that. Again I think you'll have to adapt your code to handle that, we wouldn't want to revert the code and lose the huge optimization that the last PR brought (bulk change really has a big difference 200ms vs 1100ms on 100,000 items).

@bzhjack
Copy link
Author

bzhjack commented Feb 10, 2021

ok i will adapt my code :-)
thanks

@bzhjack bzhjack closed this as completed Feb 10, 2021
@ghiscoding
Copy link
Collaborator

BTW, since you've talked about Angular... you can take a look at my lib Angular-Slickgrid, it's a wrapper on top of this SlickGrid fork which works out of the box for Angular with a lot of extra goodies.

Thanks for your issue feedback, I wasn't aware of any impact, so it's a "good to know" fact about this new strict equality.
Have a nice day

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