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

Base component may cause unexpected behavior with html_options handling #3

Open
Pagehey opened this issue Dec 9, 2023 · 0 comments · May be fixed by #4
Open

Base component may cause unexpected behavior with html_options handling #3

Pagehey opened this issue Dec 9, 2023 · 0 comments · May be fixed by #4
Assignees
Labels
bug Something isn't working

Comments

@Pagehey
Copy link

Pagehey commented Dec 9, 2023

In Vitrail::BaseComponent, initialize is:

def initialize(**html_options)
  super

  @custom_classes = html_options.delete(:class) || ""
  @html_options = html_options.stringify_keys
end

but by doing so, the html_options is mutated and key :class is removed. It could cause unexpected behavior in such examples:

<div>
  <% html_options = { class: "text-red-500", data: { some_stimulus_target: "target" } %>
  <% @elements.each do |element| %>
    <%= render Vitrail::Card.new(**html_options).with_content(element.content) %> <!-- class is not passed after first element-->
  <% end %>
</div>

In this case, after the first iteration, hash hold by html_options variable is mutated and :class key is remove.
I can see this method comes from Rails codebase but when it is done like, hash is cloned before being mutated.

Edit: I made a PR with a fix #4

@Pagehey Pagehey added the bug Something isn't working label Dec 9, 2023
@Pagehey Pagehey self-assigned this Dec 9, 2023
@Pagehey Pagehey linked a pull request Dec 9, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant