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

Components that render @inner_content generate a KeyError when no inner content is specified. #42

Closed
wrren opened this issue Feb 24, 2020 · 3 comments

Comments

@wrren
Copy link
Contributor

wrren commented Feb 24, 2020

If you define a component that renders inner content, e.g:

defmodule Card do
  use Surface.Component

  def render(assigns) do
    ~H"""
    <div class="card>
      {{ @inner_content.() }}
    </div>
    """
  end
end

but then use it without defining any wrapped content, e.g:

defmodule User do
  use Surface.Component

  def render(assigns) do
    ~H"""
    <Card />
    """
  end
end

The above will generate a KeyError in content_handler/join_contents/3, which assumes that an :inner_content assign exists.

wrren added a commit to wrren/surface that referenced this issue Feb 24, 2020
@mazz-seven
Copy link
Contributor

Shouldn't it throw an error?
I agree the error isn't informative, but I think it should throw one.

@wrren
Copy link
Contributor Author

wrren commented Feb 25, 2020

From what I've seen, it doesn't. Also I think it would be good if @inner_content was considered optional by default so that I can do things like this:

defmodule Card.Header do
  use Surface.Component

  property title, :string, required: true

  def render(assigns) do
    ~H"""
    <div class="card-header">
        <p class="card-header-title">{{ @title }}</p>
        {{ @inner_content.() }}
    </div>
    """
  end
end
defmodule User do
  use Surface.Component

  property user, :any, required: true

  def render(assigns) do
    ~H"""
    <Card>
      <Card.Header title={{ @user.display_name }} />
      {{ @user.bio }}
    </Card>
    """
  end
end

@mazz-seven
Copy link
Contributor

I was thinking about it more and you're right. It will be better that way. Also, it will work better with Slots #3 as they should be optional also.

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

2 participants