ankane / chartkick

Create beautiful JavaScript charts with one line of Ruby
https://chartkick.com
MIT License
6.33k stars 565 forks source link

Update helper to include nonce in a ViewComponent context #599

Closed multiplegeorges closed 1 year ago

multiplegeorges commented 2 years ago

Hi @ankane,

When wrapping Chartkick with ViewComponent, the helper wasn't able to determine the availability of the content_security_policy_nonce and so it wasn't including it. This was leading to CSP violations due to a bare <script> tag when a chart was rendered inside a ViewComponent.

ViewComponent puts all the normally available ActionView/Rails helpers on a local helpers proxy object when rendering, so the existing check was returning false.

This change adds an additional check in the Chartkick helper to check for ViewComponent context after all others have been checked.

About testing, I didn't want to add a ton of additional test code for such a simple addition, but open to suggestions.

Thanks for Chartkick!

multiplegeorges commented 2 years ago

After submission, I noticed another ViewComponent related issue.

ViewComponents all render in their own view_context and have no access to the parent template context. Due to this, the chart element_id was being reset to "chart-1" every time. Rendering multiple charts on the same page was impossible due to ID collisions.

The second commit makes the IDs random to avoid needing context state and to avoid collisions.

ankane commented 2 years ago

Hey @multiplegeorges, thanks for the PR. However, ViewComponent support isn't something I'd like to maintain (and the ID change breaks backwards compatibility). While a little more verbose, users who need this can probably create a method to pass default options:

def chartkick_options
  {id: ..., nonce: ...}
end

line_chart(data, **chartkick_options)