githubocto / flat-ui

MIT License
370 stars 23 forks source link

numeric strings in "id" field are coerced into numbers, then ignored for diffs despite uniqueness #3

Closed LoneRifle closed 3 years ago

LoneRifle commented 3 years ago

When calculating diffs, handleDiffDataChange() in src/store would identify a field whose number of unique values is equal to the number of data fields. The id field is assumed to be one such column... if its sort value type is a string: https://github.com/githubocto/flat-ui/blob/b9cebd6a77a1b6a350c19171bb360a8a9069eea2/src/store.ts#L138

This does not work so well when used in tandem with handleDataChange(), which generates a schema, and would coerce numeric strings into numbers. For id fields whose values are numeric strings, each identifier gets coerced into a number, which then gets ignored by handleDiffDataChange() based on the abovementioned criterion.

Because the most unique column is used as a key in diffDataMap, this in turn influences how diffs are generated.

An example of this can be found here. As id is a numeric string, the contact field is used instead, but since contact is an object type, it serves as a poor key for diffDataMap, resulting in the diffs being inaccurate in the grid.

This issue hence proposes that the id field is simply assumed to be an unique column regardless of its sort value type, or at least, if its sort value type is a string or a number. Alternatively, generateSchema() could be modified such that the data type for id remains unchanged.

Wattenberger commented 3 years ago

that's a great point, and thanks for digging through the code. I actually had it that way originally, but ran into an example that had non-unique number ids. I agree with you and expect for there to be more instances of "column ids that are numbers that we should use" than "column ids that aren't unique ids". Just pushed that update: https://flatgithub.com/datascapesg/passiton?filename=requests.json&filters=&sha=5d3ada29d53a499d9221296b0209473ee6439639&sort=id%2Cdesc&stickyColumnName=id