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

Possible regression on 2.8.0 #6143

Closed
vletoux opened this issue Mar 18, 2019 · 6 comments
Closed

Possible regression on 2.8.0 #6143

vletoux opened this issue Mar 18, 2019 · 6 comments

Comments

@vletoux
Copy link

vletoux commented Mar 18, 2019

Context

Here is a "bug report" issued, to track the issue I'm facing which can be related to Chart.js or my own configuration.
I writing it just in case someone is in the same trouble than me.

I'm developing using asp.net core in Visual studio 2017 (latest release).
I'm preparing a .js file which is the concatenation of all .js files of my solution.

Expected Behavior

When running the project, during the compilation phase, the .js file is created and minimized.

Current Behavior

The "bundle and minifier" component triggers an error about strict compilation.
No line number or source files is indicated.
The error is not triggered on 2.7.0 and an update of chartjs to 2.8.0 triggers the error.
The error is about undeclared variables "r" "g" "b" multiple times in strict mode.
The problem occured both with chart.js and chart.bundle.js

Possible Solution

I checked the source code of chart.js and the code seems to be fine.
Indeed, you can see the following code, which is fine.
var r = rgb[0]/255, g = rgb[1]/255, b = rgb[2]/255
Maybe this is a problem in the bundle and minifier (latest version in my case - https://github.com/madskristensen/BundlerMinifier)
What is strange is that my solution is using 20+ other .js files and that the problem is triggered only when incorporating the latest version of chart.js

Steps to Reproduce (for bugs)

I was not able to have a simple reproduction step.
Investigation in progress.

Environment

  • Chart.js version: 2.8.0
@benmccann
Copy link
Contributor

@vletoux since you said you "checked the source code of chart.js and the code seems to be fine", it seems to me like there's no bug in Chart.js here. We have thousands of other users and no other reports of this bug, so I think the more likely explanation is that the bug is in BundlerMinifier.

You'll really have to do a bit more leg work demonstrating where the bug lies. There's only a few of us working to maintain this project in our spare time and we already have hundreds of open bugs. It just doesn't scale for us to have to do a lot of investigation using different tools like BundlerMinifier for each new bug. We need the bug reporters to take on that burden or else it'll just be impossible to keep up with incoming bugs

If you can share more information showing this bug is in Chart.js or show how to reproduce the bug just in Chart.js without having to use other tools then we'll gladly look at it

@benmccann
Copy link
Contributor

Although it looks to me like there actually may be an issue here and you just linked to the wrong line of code. I think the issue might be in:

// http://dev.w3.org/csswg/css-color/#hwb-to-rgb
function hwb2rgb(hwb) {
  var h = hwb[0] / 360,
      wh = hwb[1] / 100,
      bl = hwb[2] / 100,
      ratio = wh + bl,
      i, v, f, n;

  // wh + bl cant be > 1
  if (ratio > 1) {
    wh /= ratio;
    bl /= ratio;
  }

  i = Math.floor(6 * h);
  v = 1 - bl;
  f = 6 * h - i;
  if ((i & 0x01) != 0) {
    f = 1 - f;
  }
  n = wh + f * (v - wh);  // linear interpolation

  switch (i) {
    default:
    case 6:
    case 0: r = v; g = n; b = wh; break;
    case 1: r = n; g = v; b = wh; break;
    case 2: r = wh; g = v; b = n; break;
    case 3: r = wh; g = n; b = v; break;
    case 4: r = n; g = wh; b = v; break;
    case 5: r = v; g = wh; b = n; break;
  }

  return [r * 255, g * 255, b * 255];
}

@vletoux
Copy link
Author

vletoux commented Mar 18, 2019

Great you found it.
This was driving me crazy when trying to find the problem.

@benmccann
Copy link
Contributor

The bug is in the color-convert library. I sent a PR to upgrade it here: chartjs/chartjs-color#6

@vletoux
Copy link
Author

vletoux commented Mar 24, 2019

I can confirm that declaring r,g,b in the hwb2rgb function does fix the problem.

@benmccann
Copy link
Contributor

Fix here: #6663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants