Support Phlex DSL on scenarios

[stephannv]

Fixes: #584

I'm not sure if this is the best solution but it's working 😅

Instead evaluating the block on Lookbook::Preview context, it is evaluating on component context, so it can render other components and use Phlex DSL.

[netlify]

✅ Deploy Preview for lookbook-docs canceled.

Name	Link
🔨 Latest commit	da802d9
🔍 Latest deploy log	https://app.netlify.com/sites/lookbook-docs/deploys/65be3d3ed76f2600071a34b1

[joeldrapper]

This does make sense if you're using Lookbook to document Phlex components that you intend to render inside other Phlex components, however, that's not the behaviour you would get from rendering a Phlex component in another context such as an ActionView view.

Perhaps it should be a configuration option for this reason. 🤔

[stephannv]

This does make sense if you're using Lookbook to document Phlex components that you intend to render inside other Phlex components, however, that's not the behaviour you would get from rendering a Phlex component in another context such as an ActionView view.

Perhaps it should be a configuration option for this reason. 🤔

Yeah, I guess I can create another preview class, eg.:
Lookbook side:

# lib/lookbook/phlex_preview.rb
module Lookbook
  class PhlexPreview < Preview
    def render(component, &block)
      super do
        component.instance_exec component, &block
      end
    end
  end
end

<!-- app/views/lookbook/previews/preview.html.erb -->
<% if defined?(Phlex::SGML) && @render_args[:component].is_a?(Phlex::SGML) %>
  <%= render @render_args[:component], &@render_args[:block] %>
...

User side:

class MyComponentPreview < Lookbook::PhlexPreview # instead of Lookbook::Preview
  def default
    render MyComponent.new do |c|
      c.title { "Hello" }
      h2 { "World" }
      render MyOtherComponent.new
    end
  end
end

[stephannv]

I'm not sure why this was raw + call, but changing to render component, args, block worked on specs and on my project.
Using render Rails calls component.render_in(self, &block). Ref: https://github.com/rails/rails/blob/main/actionview/lib/action_view/helpers/rendering_helper.rb#L42

[allmarkedup]

@stephannv the discussion in #400 should help explain the rationale for this, ugly as it is!

[joeldrapper]

Having a different preview class for Phlex-context-previews feels better to me, but rather than instance-exec’ing the the block against the component being rendered, it would be better to have the preview itself be a Phlex::HTML object. That’s more consistent with reality.

It’s possible for components to override HTML elements.

class A < Phlex::HTML
  def template
    h1 { "Hello" }
    yield
  end

  def h1 = super(class: "from_a")
end

class B < Phlex::HTML
  def template
    render A.new do
      h1 { "World!" }
    end
  end
end

Rendering B should output the following because the first heading came from A and the second heading came from B.

<h1 class="from_a">Hello</h1><h1>World!</h1>

But if B was a preview object using instance_exec on A, it would output this instead

<h1 class="from_a">Hello</h1><h1 class="from_a">World!</h1>

We can’t make Lookbook::PhlexPreview inherit from Phlex::HTML because it needs to inherit from Lookbook::Preview, but the next version of Phlex will provide everything from Phlex::HTML as a module you can include instead. It’s probably worth waiting on that.

[stephannv]

Having a different preview class for Phlex-context-previews feels better to me, but rather than instance-exec’ing the the block against the component being rendered, it would be better to have the preview itself be a Phlex::HTML object. That’s more consistent with reality.

It’s possible for components to override HTML elements.

class A < Phlex::HTML
  def template
    h1 { "Hello" }
    yield
  end

  def h1 = super(class: "from_a")
end

class B < Phlex::HTML
  def template
    render A.new do
      h1 { "World!" }
    end
  end
end

Rendering B should output the following because the first heading came from A and the second heading came from B.

<h1 class="from_a">Hello</h1><h1>World!</h1>

But if B was a preview object using instance_exec on A, it would output this instead

<h1 class="from_a">Hello</h1><h1 class="from_a">World!</h1>

It's true, I could reproduce this problem now =(.

We can’t make Lookbook::PhlexPreview inherit from Phlex::HTML because it needs to inherit from Lookbook::Preview, but the next version of Phlex will provide everything from Phlex::HTML as a module you can include instead. It’s probably worth waiting on that.

Nice, I did this on my similar lib (Blueprint) and I think it's a great approach

[allmarkedup]

@stephannv thanks for taking the time to put this together. And @joeldrapper thanks for jumping in on the discussion with your expertise on this, much appreciated. I'm sadly not using Phlex on a day-today basis at the moment so forgive me if i'm a bit out of my depth with the details of what is being discussed here.

I'm certainly not opposed to having a Phlex-specific preview class to inherit from (or a module that can be included in preview classes for Phlex components) if that means previews can be written in a way that feels more natural for Phlex users.

FWIW, the sub-component render issue is actually also present for ViewComponent previews - to render a sub-component within a parent's component's block context you need to use the parent component's render method. You have to do something like:

def example_preview
  render ExampleComponent.new do |parent|
    parent.render ChildComponent.new
  end
end

This is how it works in ViewComponent's own preview system which Lookbook previews are based on (and aim to be 100% compatible with). It's definitely not ideal for all the reasons you mention in #584 but will likely have to stay like that (for VC component previews, anyway).

So is the general feeling here that it's worth waiting until Phlex::HTML (or the contents thereof) is made available as an module before taking this any further?

[joeldrapper]

So is the general feeling here that it's worth waiting until Phlex::HTML (or the contents thereof) is made available as an module before taking this any further?

I think that's our best option. It shouldn't be too long now.

[cirdes]

@stephannv helped me out, and I was able to make it work by using an initializer:

config/initializers/lookbook.rb

module Lookbook::PreviewOverrides
  # see https://github.com/ViewComponent/lookbook/issues/584
  def render(component = nil, **args, &block)
    if block
      super { component.instance_exec component, &block }
    else
      super
    end
  end
end

Rails.application.configure { Lookbook::Preview.prepend Lookbook::PreviewOverrides }

But I would love to see better Phlex support!