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

[BUG] Time chart with moment.js breaks in Firefox extension #5901

Closed
jeroenheijmans opened this issue Dec 8, 2018 · 3 comments · Fixed by #5904
Closed

[BUG] Time chart with moment.js breaks in Firefox extension #5901

jeroenheijmans opened this issue Dec 8, 2018 · 3 comments · Fixed by #5904
Milestone

Comments

@jeroenheijmans
Copy link

jeroenheijmans commented Dec 8, 2018

I'm trying to use Chart.js in combination with Moment.js inside a browser extension. Things load fine inside my dummy test html page in Chrome and Firefox, and as part of the extension when loaded in Chrome. In Firefox however, nothing is rendered.

Basically, I'm reporting the same symptoms as this user on Stack Overflow, but this time with a full repro.

I presume (though don't know for sure) that this is a bug with Chart.js.

Expected Behavior

A chart with type: "time" axis should render properly, also when packed as a Firefox extension.

Here's how it should look, as it does in Chrome:

Chrome

Current Behavior

Instead, only an empty area where the chart should be is displayed.

Here's what it looks like in Firefox:

Firefox

Possible Solution

No known workaround or solution.

If you remove the type: "time" part from the code, a graph is shown (but in my actual scenario it doesn't look nice, as I have dense time-based data).

Steps to Reproduce

I've made a self-contained repository with steps to reproduce, repeated here:

  1. Clone the repo.
  2. Open Firefox, go to about:debugging.
  3. Choose "Load Temporary Add-on..." and pick the manifest.json file
  4. In a new tab, go to https://github.com/* where the extension does its magic

For completeness, repeating the important bits here, starting with the chart definition:

const element = document.body.insertBefore(document.createElement("canvas"), document.body.firstChild);
const chart = new Chart(element.getContext("2d"), {
  type: "line",
  data: {
    datasets: [{
        label: "Test Dataset",
        data: [
          { x: moment('20180403'), y: 3 },
          { x: moment('20180404'), y: 8 },
          { x: moment('20180405'), y: 6 },
        ]
      },
    ],
  },
  options: { scales: { xAxes: [{ type: "time" }] } },
});

and the extension manifest:

{
  "manifest_version": 2,
  "name": "Repro ChartJS Issue",
  "short_name": "ReproChartJsIssue",
  "version": "1.0.0",
  "author": "Jeroen Heijmans",
  "description": "Extension solely meant for reproducing an issue with ChartJS",
  "content_scripts": [
      {
          "matches": ["https://github.com/*"],
          "js": ["moment.min.js", "Chart.min.js", "app.js"],
          "run_at": "document_end"
      }
  ]
}

Context

I have a Chrome extension which is open source and I would like to pack it into a Firefox extension as well.

PS. In this Stack Overflow comment it is suggested that it's a Firefox bug (2 years ago, should be fixed by now), but I don't think it is. The reason: that bug was with incorrect loading of scripts, and if I trigger that situation (by giving manifest.json the wrong order of scripts), then Chart.js properly notifies me in the console of this error.

PPS. To further exclude mentioned Firefox loading-order bug as the cause I also inlined both libraries in my app.js file in the extension, basically like so:

(function() {
  // minified moment.js code here...
  setTimeout(() => {
    // minified chart.js code here
    // application code creating the chart here
  }, 1000);
}());

But to no avail: same negative outcome.

Environment

@jeroenheijmans
Copy link
Author

I dug some further, and debugged an unminified version of Chart.js, breaking on all exceptions. It does seem after all that moment is not available to Chart.js (even though my app.js has it available). This might be an indication that my bug report is a false positive, but it might also be an indication that the way Chart.js is requiring scripts like momentjs is not compatible with Mozilla's way of separating scripts in a plugin from eachother.

I found that these Mozilla docs to some degree describe the logic behind my scenario.

Based on that same content, I found a workaround: appending window.moment = moment; to the minified momentjs code that is packed into the plugin seems to "fix" things.

@simonbrunel
Copy link
Member

simonbrunel commented Dec 9, 2018

Good timing: I'm currently investigating if we could switch our build process to use rollup (see this comment) and I think your issue is caused by how Browserify doesn't handle external dependencies and this line of code. So you could checkout my branch and see if it fixes your issue (don't use the ESM build).

@jeroenheijmans
Copy link
Author

@simonbrunel I've cloned your repository, checked out the branch, installed dependencies, ran the build, and used the resulting Chart.min.js file in my repro: it works!

PS. Personally I'd call it "Bad timing" 😅, with your work released I wouldn't have been stuck for a few hours 😄. But that's life! Thx for the great work on the lib.

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

Successfully merging a pull request may close this issue.

2 participants