Match structure of all data points so each DataSeries contain the same keys

[envex]

What does this implement/fix?

https://6062ad4a2d14cd0021539c1b-jcjslbrrgq.chromatic.com/?path=/story/polaris-viz-charts-barchart-playground--mis-matched-data

Now that we're allowing consumers to create any type of report we were running into an issue where data being fed into BarChart didn't have the same data between each DataSeries, which would cause the chart to crash.

For example, data would come in like:

[
      {
        name: 'Canada',
        data: [
          {key: 'Mice', value: 13.28},
          {key: 'Dogs', value: 23.43},
          {key: 'Cats', value: 6.64},
          {key: 'Birds', value: 54.47},
        ],
      },
      {
        name: 'United States',
        data: [
          {key: 'Lizards', value: 350.13},
          {key: 'Turtles', value: 223.43},
          {key: 'Mice', value: 15.38},
          {key: 'Snakes', value: 122.68},
          {key: 'Dogs', value: 31.54},
          {key: 'Birds', value: 94.84},
        ],
      },
      {
        name: 'China',
        data: [
          {key: 'Snakes', value: 0},
          {key: 'Dogs', value: 0},
        ],
      },
    ]

Now we're going to fill all the data so each DataSeries has key/values that match the entire data set, not just the single DataSeries itself.

[github-actions]

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
polaris-viz-core-cjs	61.3 KB (0%)	1.3 s (0%)	2.3 s (+52.11% 🔺)	3.5 s
polaris-viz-cjs	211.24 KB (+0.06% 🔺)	4.3 s (+0.06% 🔺)	4.3 s (+41.14% 🔺)	8.5 s
polaris-viz-esm	173.61 KB (+0.06% 🔺)	3.5 s (+0.06% 🔺)	2.1 s (+1.75% 🔺)	5.5 s
polaris-viz-css	4.57 KB (0%)	92 ms (0%)	429 ms (-13.55% 🔽)	520 ms
polaris-viz-esnext	178.71 KB (+0.07% 🔺)	3.6 s (+0.07% 🔺)	2.4 s (-1.31% 🔽)	6 s

[envex]

I'm not sure if this is even worth testing, but the original implementation below would get between 80 and 100ms. The new implementation gets around 4ms.

for (const {data} of dataSeries) {
  for (const {key} of data) {
    allKeys.add(`${key}`);
  }
}

return dataSeries.map(({name, data}) => ({
  name,
  data: [...allKeys].map((key) => {
    const matchedValue = data.find((item) => item.key === key);
    return matchedValue ? matchedValue : {key, value: null};
  }),
}));

[maryamkaka]

Relying on this check to see if we need to fill in missing points is going to miss cases where the dataSeries is the same length but the keys are different. I think we'll want to check that the keys are the same across each series instead

e.g. This is a valid case where we'd want to continue but would early return

  {
    name: 'Canada',
    data: [
      {key: 'Cats', value: 6.64},
      {key: 'Birds', value: 54.47},
    ],
  },
  {
    name: 'United States',
    data: [
      {key: 'Lizards', value: 350.13},
      {key: 'Turtles', value: 223.43},
    ],
  },
];

[envex]

Right, I thought about that but I'm not sure how often we'd hit this case.

I was originally doing an equality check on the next array in this .some() but it adds about 10ms onto the time.

I wonder if we should just always run the code and not try and bail early.

[bencmilton]

I wonder if we should just always run the code and not try and bail early.

Agreed, probably worth just running it every time. Looking for extraneous keys would be a hard one to check too

  {
    name: 'Canada',
    data: [
      {key: 'Cats', value: 6.64},
      {key: 'Birds', value: 54.47},
    ],
  },
  {
    name: 'United States',
    data: [
      {key: 'Cats', value: 350.13},
      {key: 'Birds', value: 223.43},
      {key: 'Snakes', value: 100.00},
    ],
  },
];

[maryamkaka]

hmm ... I always just assume if there is a data edge case there are going to be merchant out there that are going to see it 🤷‍♀️

I'm team lets not bail early and we can always come back and optimize it later. Curious what others think tho

[envex]

Sounds good, I removed the early bailout

[bencmilton]

Looks good to me 🚀