Skip to content

Allow defining layout in callback function#1808

Open
mitchellhenke wants to merge 1 commit intonaymspace:developfrom
mitchellhenke:runtime-layout
Open

Allow defining layout in callback function#1808
mitchellhenke wants to merge 1 commit intonaymspace:developfrom
mitchellhenke:runtime-layout

Conversation

@mitchellhenke
Copy link

Hello!

I was debugging some compile cycles in a Phoenix application, and noticed that backpex was part of the issue.

The demo application has a good example of what I'm running into. If you run mix xref graph --format cycles --label compile-connected, it reports that there is a compile cycle:

demo/ $ mix xref graph --format cycles --label compile-connected
1 cycles found. Showing them in decreasing size:

Cycle of length 16 (9 compile, 1 export):

    lib/demo_web/components/core_components.ex
    lib/demo_web/components/layouts.ex (export)
    lib/demo_web/controllers/redirect_controller.ex
    lib/demo_web/endpoint.ex
    lib/demo_web/item_actions/user_soft_delete.ex
    lib/demo_web/live/address_live.ex (compile)
    lib/demo_web/live/category_live.ex (compile)
    lib/demo_web/live/film_review_live.ex (compile)
    lib/demo_web/live/invoice_live.ex (compile)
    lib/demo_web/live/post_live.ex (compile)
    lib/demo_web/live/product_live.ex (compile)
    lib/demo_web/live/short_link_live.ex (compile)
    lib/demo_web/live/tag_live.ex (compile)
    lib/demo_web/live/user_live.ex (compile)
    lib/demo_web/resource_actions/upload.ex
    lib/demo_web/router.ex

This results in a change in any of the files recompiling all of the files, which usually means slower compile times during development in apps that use Backpex.

I'm not deeply familiar with Elixir compiler functionality, but my understanding is the reason for it is the DemoWeb.Layouts becomes a compile dependency of the LiveViews when specified in use Backpex.LiveResource. DemoWeb.Layouts includes use DemoWeb, :html which ends up pulling in the router, etc.

To continue the above example using the demo app, if you apply this patch to replace the layout option with the callback, the demo app no longer has a compile cycle:

demo/ $ mix xref graph --format cycles --label compile-connected
No cycles found

I'm proposing allowing specifying the layout as a callback function to avoid the specific compile dependency between layouts and the LiveView. To avoid a breaking change, I've not removed the current option. I also haven't tried updating any guides, documentation, or similar, but am happy to if you're open to this change.

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

Successfully merging this pull request may close these issues.

1 participant