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

Class instances not handled properly in configuration #7340

Closed
iddings opened this issue May 11, 2020 · 1 comment · Fixed by #7381
Closed

Class instances not handled properly in configuration #7340

iddings opened this issue May 11, 2020 · 1 comment · Fixed by #7381

Comments

@iddings
Copy link

iddings commented May 11, 2020

Expected Behavior

Class instances should be preserved in chart options, including instance methods.

Current Behavior

Only an object's own enumerable properties (returned by Object.keys) are preserved.
helpers.merge appears to be at fault here, as instance methods are not enumerable properties.

Possible Solution

Cloning an instance of a class is possible with something like:

Object.assign( Object.create( original ), original );

This would only be a shallow copy, but that seems reasonable in the context of a class instance.

Steps to Reproduce (for bugs)

class MyConfigClass {
  
  constructor(parameter) {
    this._private_parameter = parameter;
  }
  
  getParameter() {
    return this._private_parameter;
  }
  
}

  
Chart.plugins.register({
  id: 'example',
  beforeInit: function(chartInstance, pluginOptions) {

    // expect: an instance of MyConfigClass
    // actual: { _private_parameter: 'a value' }
    console.log(pluginOptions);

  }
});

const chart = new Chart(document.querySelector("#chart"), {
  options: {
    plugins: {
      'example': new MyConfigClass("a value")
    }
  }
});

CodePen

Context

Our use case is displaying climate data in either UTC, or local standard time (of the station at which the data was collected). We essentially pretend that DST doesn't exist, so we use fixed offsets from UTC to present data.

CodePen

Without a workaround, passing an instance of FixedOffsetZone results in no data being displayed (because Luxon sees the object it is passed as an invalid zone).

This example has a relatively easy workaround (i.e. using a number instead of a FixedOffsetZone instance), however, in our actual use case, we're accepting any valid Luxon zone (string, number, or any instance that implements luxon.Zone) from an external source, and passing it to the adapter options in Chart.js. This makes it impossible to convert every possible zone to something that Chart.js can handle, as the external source could pass in an instance of a custom class which implements luxon.Zone.

Environment

  • Chart.js version: 2.9.3
  • Browser name and version: Chrome 81.0.4044.129
  • Link to your project: N/A
@kurkle
Copy link
Member

kurkle commented Mar 7, 2021

@kurkle kurkle closed this as completed Mar 7, 2021
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.

4 participants