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

Cannot use some Phoenix helpers within curly braces #70

Closed
mathewdgardner opened this issue Apr 17, 2020 · 8 comments
Closed

Cannot use some Phoenix helpers within curly braces #70

mathewdgardner opened this issue Apr 17, 2020 · 8 comments

Comments

@mathewdgardner
Copy link
Contributor

mathewdgardner commented Apr 17, 2020

Cannot use Phoenix HTML helpers with do blocks with {{ }}.

Environment

  • Elixir version: 1.10.2
  • OTP version: 22.3.2
  • Phoenix version: 1.4.16
  • Phoenix LiveView version: 0.11.0
  • Operating system: macOS Catalina 10.15.4

Contrived Example

defmodule Foo do
  use Surface.Component
  import Phoenix.HTML.Link

  property links, :list, default: ["foo", "bar", "baz"]

  def render(assigns) do
    ~H"""
    <div>
      {{ for l <- @links do }}
        {{ link to: "#" do }}
          <span>{{ l }}</span>
        {{ end }}
      {{ end }}
    </div>
    """
  end
end

With this we get a compiler warning and error

Compiling 1 file (.ex)
warning: unexpected beginning of EEx tag "<%=" on end of expression "<%= end %>", please remove "=" accordingly
  lib/foo.ex:14


== Compilation error in file lib/foo.ex ==
** (SyntaxError) lib/foo.ex:14: unexpected token: end
    lib/eex/compiler.ex:101: EEx.Compiler.generate_buffer/4
    lib/eex/compiler.ex:54: EEx.Compiler.generate_buffer/4
    expanding macro: Surface.sigil_H/2
    lib/foo.ex:9: Parent.render/1

In this contrived example (intentionally not using the :for directive) we can move past the error by manually creating the anchor tag, but is this desired behavior?

If we remove the do block for link/2 we lose the error but keep the warning.

Sorry @msaraiva, I don't mean to pummel you with issues. This is great package and I can't wait to see it reach v1.0.0!

@zamith
Copy link
Contributor

zamith commented Apr 17, 2020

Since you're not really taking advantage of Surface (at from the example), have you tried using the Raw component?

defmodule Foo do
  use Surface.Component
  import Phoenix.HTML.Link

  alias Surface.Components.Raw

  property(links, :list, default: ["foo", "bar", "baz"])

  def render(assigns) do
    ~H"""
    <div>
      <#Raw>
        <%= for l <- assigns[:links] do %>
          <%= link to: "#" do %>
            <span><%= l %></span>
          <% end %>
        <% end %>
      </#Raw>
    </div>
    """
  end
end

There is still the issue that Surface interpolates everything with = which maybe @msaraiva is addressing in #15, but it should solve your particular problem.

@mathewdgardner
Copy link
Contributor Author

mathewdgardner commented Apr 17, 2020

@zamith This does get around it like manually using <a>...</a> does. However, in this convoluted example I'd prefer the later over the former.

I guess I'm really just wondering if there is any compatibility between Surface and all the helpers we know and love. It seems the answer is yes with the use of #Raw but that is not intuitive or obvious to newcomers.

@msaraiva
Copy link
Member

Just like React, Surface cannot accept partial/incomplete expressions and probably never will since it has to generate a valid tree structure of the HTML/Components nodes at compile-time. So we can only accept complete expressions that can be be fully assigned to a particular node/attribute.

I have some further comments about this here #23 (comment).

In general, you should avoid having too much logic in your template. In case you do, and the available directives (:for, :if, etc.) are not enough, it's probably a smell and a sign that the logic is getting too complex and you should consider moving that code to someplace else.

If for any particular reason you really need to use something like <% ... %>, just create a separate function and use ~L or ~E. You can call that function directly in your Surface template using {{}}.

<#RAW> can be an alternative as well, however I would first investigate the complexity of the logic and see if it could be moved to a better place before going that road.

@mathewdgardner
Copy link
Contributor Author

mathewdgardner commented Apr 17, 2020

I ran into this converting some partials into surface components. The complexity was just in creating a link using a do block so that some inner html could be set. My contrived example is just that, contrived, to illustrate it.

I suppose it just means there should be another <Link> component that wraps the use of phoenix helpers?

@msaraiva
Copy link
Member

I suppose it just means there should be another component that wraps the use of phoenix helpers?

Exactly! Maybe we can come up with an alternative in the future to make this work somehow, but for now, our only options are: move to a separate function or create a wrapper component. The goal is to create builtin components for the most common helpers so we can provide a nice development experience.

Sorry @msaraiva, I don't mean to pummel you with issues. This is great package and I can't wait to see it reach v1.0.0!

No worries. That's why I'm here for :) I'm glad you're enjoying.

@msaraiva
Copy link
Member

@mathewdgardner feel free to bring any real case you already needed (like Link). This kind of feedback is important so we can decide which builtin components should be added first.

@mathewdgardner
Copy link
Contributor Author

@msaraiva Good deal, I have several issues I've kept track of as I've learned how to use Surface. I can keep posting them in separate issues to keep them contained if they are welcome.

I can also put together a PR for a link component.

@msaraiva
Copy link
Member

Awesome! PRs are always welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants