Skip to content

Output a warning whenever user tries to use datasets without a unique key #135

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 29, 2018
Merged

Output a warning whenever user tries to use datasets without a unique key #135

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b970cdd
Output a warning whenever user tries to use datasets without a unique…
BenLorantfy May 24, 2017
70e91d9
grammar fix
BenLorantfy May 24, 2017
ea08fa1
only show warning in dev enviroment
BenLorantfy Jul 7, 2017
5ba1867
Merge remote-tracking branch 'origin/master' into ben-master
BenLorantfy Feb 3, 2018
6802aa5
Rebased, changed NODE_ENV to get from process.env, moved code to chec…
BenLorantfy Feb 3, 2018
9a6d79b
linting fixes
BenLorantfy Feb 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import isEqual from 'lodash.isequal';

class ChartComponent extends React.Component {
static getLabelAsKey = d => d.label;

static propTypes = {
data: PropTypes.oneOfType([
PropTypes.object,
Expand Down Expand Up @@ -155,8 +155,14 @@ class ChartComponent extends React.Component {
// use the key provider to work out which series have been added/removed/changed
const currentDatasetKeys = currentDatasets.map(this.props.datasetKeyProvider);
const nextDatasetKeys = nextDatasets.map(this.props.datasetKeyProvider);
const shouldWarn = !currentDatasetKeys.every(d => typeof d !== "undefined") || !nextDatasetKeys.every(d => typeof d !== "undefined");
const newDatasets = nextDatasets.filter(d => currentDatasetKeys.indexOf(this.props.datasetKeyProvider(d)) === -1);

if (shouldWarn && !this.hasWarned) {
Copy link
Collaborator

@jerairrest jerairrest May 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should only occur if you are not in a production build. :)

Copy link
Contributor Author

@BenLorantfy BenLorantfy Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I check if production vs. development?

Copy link
Contributor Author

@BenLorantfy BenLorantfy Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this will work unless you have a better method https://stackoverflow.com/a/25922668

this.hasWarned = true; // Only warn once per chart so console isn't spammed with warnings
console.error('[react-chartjs-2] Warning: Each dataset needs a unique key. By default, the "label" property on each dataset is used. Alternatively, you may provide a "datasetKeyProvider" as a prop that returns a unique key.');
}

// process the updates (via a reverse for loop so we can safely splice deleted datasets out of the array
for (let idx = currentDatasets.length - 1; idx >= 0; idx -= 1) {
const currentDatasetKey = this.props.datasetKeyProvider(currentDatasets[idx]);
Expand Down