surface-ui / surface

A server-side rendering component library for Phoenix
https://surface-ui.org
MIT License
2.08k stars 150 forks source link

add support to export default when use a single hook #636

Closed herminiotorres closed 2 years ago

herminiotorres commented 2 years ago

Setting directive :hook without a value should set its value to "default".

Close this issue https://github.com/surface-ui/surface/issues/631

herminiotorres commented 2 years ago

Hey @paulstatezny

I working on this implementation, and when I run mix format, the format change:

<div :hook></div>

into this:

<div :hook={true}></div>

And @msaraiva and I, take a look at the surface format, and we found working around to not format this directive, since :hook is not a boolean directive and this implementation brings us a new situation never pass before when this directive could assume as a default hook when doesn't have any value.

msaraiva commented 2 years ago

@paulstatezny for now, we could add a new clause:

  defp render_attribute({":hook", true, _meta}, _opts),
    do: "#{name}"

above this one:

https://github.com/surface-ui/surface/blob/0f49505ee8a4535919d88ca7db7c62d4a29aa44d/lib/surface/formatter/phases/render.ex#L334

Then later we could implement a more generic long-term solution that would be to store in the metadata the information about the original format (e.g. :dir vs :dir={true}) so we can properly format any directive or attribute.

WDYT?

paulstatezny commented 2 years ago

Thanks for the contribution @herminiotorres :)

@msaraiva I agree with your suggestion completely. We should add that clause that targets :hook until the metadata includes information to handle it more generically.

msaraiva commented 2 years ago

@herminiotorres in this case, I believe it's better to implement this small change in the formatter in the scope of this PR.

msaraiva commented 2 years ago

@herminiotorres thank you! ❤️