Chart.js icon indicating copy to clipboard operation
Chart.js copied to clipboard

Safari 12.1.2 getting error Unexpected token '='. Expected an opening '(' before a method's parameter list

Open apunjaval opened this issue 2 years ago • 6 comments

Expected behavior

All the charts should work on older safar browsers like 12.1.12``

Current behavior

Using Tree shaking to build using "npm run build" the donut chart works in latest safari but does not work on version safari 12.1.12

issue is similar to #11028 , but it is not working on chartjs 4.3.0 using below code

vi ./src/acquisitions.js

import { Chart, Colors, BarController, CategoryScale, LinearScale, BarElement, DoughnutController, ArcElement, Legend, Title } from 'chart.js'

Chart.register( Colors, BarController, BarElement, CategoryScale, LinearScale, DoughnutController, ArcElement, Legend, Title ); export {Chart}

vi src/index.html

Chart.js example
<!-- <script type="module" src="dimensions.js"></script> -->
<script type="module" src="acquisitions.js"></script>
<script type="module">

import {Chart} from './acquisitions.js'
(async function() { const data = [ { year: 2010, count: 10 }, { year: 2011, count: 20 }, { year: 2012, count: 15 }, { year: 2013, count: 25 }, { year: 2014, count: 22 }, { year: 2015, count: 30 }, { year: 2016, count: 28 }, ];

new Chart( document.getElementById('acquisitions'), { type: 'doughnut', data: { labels: ["Africa", "Asia", "Europe", "Latin America", "North America"], datasets: [ { label: "Population (millions)", backgroundColor: ["#3e95cd", "#8e5ea2","#3cba9f","#e8c3b9","#c45850"], data: [2478,5267,734,784,433] } ] }, options: { title: { display: true, text: 'Predicted world population (millions) in 2050' } } } ); })();

then i run npm run build

Reproducible sample

using above code mentioned

Optional extra steps/info to reproduce

home/akshaypunja/.nvm/versions/node/v20.3.1/lib ├── [email protected] ├── [email protected] ├── [email protected] ├── [email protected] └── [email protected]

Possible solution

what changes i should do to ./src/acquisitions.js so that it supports older browser

Context

No response

chart.js version

v4.3.0

Browser name and version

Safari 12.1.12

Link to your project

No response

apunjaval avatar Jul 16 '23 16:07 apunjaval

used babel to support older browsers and polyfill libraries

apunjaval avatar Jul 18 '23 17:07 apunjaval

Hi, I have the same problem. I load chart.j dinamically in an angular app

manduriomane avatar Jul 25 '23 10:07 manduriomane

Right.. we can remove the comment, but aren't we violating the license by doing it?

MIT license states:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

it's really weird that the comment ends up between new and class, can't chart.js fix it? shouldn't this be reopened?

I tried babel comment: false, no effect, tried browserslist compatibility and no effect.. but even if I find a way to remove the comments, I don't think I can legally do it... (and I think its intentional for bundlers to maintain /*! comments)

cc @LeeLenaleee for the license question and maybe reopen? (hopefuly not spamming you 🙏 ), I think the fix should be in the bundler but... babel? webpack? not even sure what transforms that source in that way...

Grohden avatar Apr 08 '24 19:04 Grohden

I've added a unique comment to each license comment to find it, here's it compiled in the chunk: (the one that safari cries about) image

it comes from chart.js/dist, this part here: image

from what I see, line 147 in my sources seems to be where var animator = new Animator() is called... seems that something is inlining the class somehow and putting the license comment between the new and the inlined class? wtf

Grohden avatar Apr 08 '24 20:04 Grohden

Re opend the issue but I won't be able to answer your question about the license, I don't have suffiencient knowledge about that

LeeLenaleee avatar Apr 08 '24 22:04 LeeLenaleee

I've just realized that I've opened too many tabs, took too much coffee, got things mixed and thought that the license comment was the issue (also thought I saw someone commenting about the license comment here...). The real issue is lack of compiling to a supported syntax really...

It took me waaay too much time but here's exactly where the syntax error occurs:

I had to:

  • create a next 13 project (the comment moves if you use next 14, not sure if they fixed the issue, will test later)
  • add chart.js 4.4.2
  • compile it (npm run build)
  • check the .next folder for the comment header in any chunk
  • pretty format the chunk so safari points out the exact line where we have the issue
  • open safari 13 (I'm using browserstack)
  • open a reverse proxy (tried ngrok.. and surprisingly, their warning page has this syntax error, so I had to use localtunnel instead)
  • finally access the site with safari 13 and get what safari points out as the issue image

So the path here is to compile with babel or anything to support the syntax.

@LeeLenaleee thx for at least reopening, one last question before closing.. shouldn't the dist folder have the support for the mentioned syntax by default? I don't think babel tries to transform node_module packages by default...

edit: for nextjs, it seems transpilePackages: ['chart.js'] in next.config should do the trick

Grohden avatar Apr 11 '24 13:04 Grohden