SalesforceLabs / LightningWebChartJS

Simple yet flexible charting Lightning Web Component using Chart.js for admins & developers
https://sfdc.co/lwcc
MIT License
143 stars 61 forks source link

Charts are broken and not rendered with LWS turned on #106

Open Manish-Padakanti opened 2 years ago

Manish-Padakanti commented 2 years ago

Who is the bug affecting?

Consumers of LightningWebChartJS repo individually with custom deploy of components.

What is affected by this bug?

Some of the Charts are broken and not rendered

When does this occur?

After enabling LWS(Lightning Web security) on the org.

Where on the platform does it happen?

Where c-chart LWC components are used.

How do we replicate the issue?

Enable setting Use Lightning Web Security for Lightning web components from Setup -> Session settings

Expected behavior (i.e. solution)

All Charts should work with LWS turned on.

Other Comments

Problem: sanitize method in utils lwc

image

victorgz commented 2 years ago

Hi @Manish-Padakanti , thanks for your feedback and for using the solution !

I am running some tests in an org with LWS enabled and the charts (Chart Builder and custom chart) are working for me. I've also run some ESLint tests for LWS and getting nothing...

Can you please share the code of the chart you're trying to use? That will help us investigate the problem.

Thanks !

Manish-Padakanti commented 2 years ago

Here's a simple example that fails.

export default class TestContainer extends LightningElement {
  hb = {
    bgColor: '#C7F296',
    detail: [10, 20, 15, 5, 25, 30],
    label: 'No of items',
  };
}
<c-thunder-chart type="horizontalBar">
  <c-dataset labels='["Group 1", "Group 2"]'>
    <c-data
      label={hb.label}
      detail={hb.detail}
      bordercolor={hb.bgColor}
      backgroundcolor={hb.bgColor}
    ></c-data>
</c-dataset>
<c-title
  text="Horizontal Bar Chart"
  fontfamily="'Salesforce Sans', Arial, sans-serif"
  fontcolor="rgb(8, 7, 7)"
></c-title>
</c-thunder-chart>
victorgz commented 2 years ago

Hi @Manish-Padakanti ,

I've successfully reproduced the error from my side, but unfortunately we cannot do anything as the error with LWS comes from the Chart.js library we're referencing.

You mentioned in the description that the problem comes from the sanitze function. I've tested in the LWS Console and the function works fine with LWS enabled.

We've also incuded new ESLint validations for LWS (#107) and our components are compliant. However, the error returned "Invalid mutation: Cannot define property _chartjs on..." comes from the library (Chart.js 2.8.0) included as a static resource.

I am going to update the FAQ in the documentation to mention this case, but for now there's nothing we can do. We're planning to upgrade the library to make it work with the latest version of Chart.js (3.x). We cannot guarantee that the new version will fix the LWS issue, and we don't have neither an ETA for this upgrade (it requires a lot of rework of the solution).

Sorry for the inconveniences and thanks for using the solution !

user851 commented 1 year ago

Chart JS version 4.3 confirmed working with LWS enabled. This project appears abandoned, is there anyone actively working on it anymore?

victorgz commented 1 year ago

Hi @user851, thanks for the feedback on LWS, that's actually good news ! For information, we're still maintaining the project (you can have a look at the latest issues) with the Chart JS version it was implemented initially. We've thought about migrating to a new Chart JS version, but honestly there's a lot of work to do. It wouldn't be only to replace the library but we would also need to :

We'll re-think about it anyway. Thanks again!