-
Notifications
You must be signed in to change notification settings - Fork 77
Allow app web helpers in Backpex extensions #2170
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,3 +63,47 @@ defmodule MyAppWeb.PostLive do | |
| end | ||
| end | ||
| ``` | ||
|
|
||
| ## Backpex extension modules now import `Phoenix.Component` | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should move to 0.20 |
||
|
|
||
| The `BackpexWeb` entrypoints for item actions, filters, and metrics now import | ||
| `Phoenix.Component` instead of using it. This avoids registering the | ||
| declarative `@before_compile` hook twice when an extension also uses its application's | ||
| HTML entrypoint. | ||
|
|
||
| You can now use application-level Gettext, components, and verified routes directly in | ||
| these extensions, in either order: | ||
|
|
||
| ```elixir | ||
| defmodule MyAppWeb.ItemActions.Archive do | ||
| use BackpexWeb, :item_action | ||
| use MyAppWeb, :html | ||
|
|
||
| # ... | ||
| end | ||
| ``` | ||
|
|
||
| This also applies to custom filters using `BackpexWeb, :filter` or one of | ||
| `Backpex.Filters.Select`, `Backpex.Filters.MultiSelect`, `Backpex.Filters.Boolean`, and | ||
| `Backpex.Filters.Range`, as well as metrics using `BackpexWeb, :metric`. | ||
|
|
||
| No change is required for extensions that only use `~H` and the helpers provided by | ||
| Backpex. If an item action, filter, or metric declares its own function components with | ||
| `attr` or `slot`, it must now additionally `use MyAppWeb, :html` or | ||
| `use Phoenix.Component`. | ||
|
|
||
| ## Custom fields can use the host LiveComponent entrypoint | ||
|
|
||
| `Backpex.Field` still needs the complete `Phoenix.LiveComponent` setup. To make your | ||
| application's Gettext backend, components, and verified routes available without using | ||
| `Phoenix.Component` twice, configure the host LiveComponent entrypoint: | ||
|
|
||
| ```elixir | ||
| use Backpex.Field, | ||
| config_schema: @config_schema, | ||
| live_component: {MyAppWeb, :live_component} | ||
| ``` | ||
|
|
||
| The configured entrypoint must set up a `Phoenix.LiveComponent`, as the standard | ||
| Phoenix-generated `MyAppWeb, :live_component` entrypoint does. It replaces Backpex's | ||
| default LiveComponent setup, so do not additionally `use MyAppWeb, :html` in the field. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,15 +226,35 @@ defmodule Backpex.Field do | |
|
|
||
| @doc """ | ||
| Defines `Backpex.Field` behaviour and provides default implementations. | ||
|
|
||
| A custom field can use its application's LiveComponent entrypoint to make | ||
| application helpers such as Gettext and verified routes available: | ||
|
|
||
| use Backpex.Field, | ||
| config_schema: @config_schema, | ||
| live_component: {MyAppWeb, :live_component} | ||
|
|
||
| The configured entrypoint must set up a `Phoenix.LiveComponent`, as the standard | ||
| Phoenix-generated `MyAppWeb, :live_component` entrypoint does. | ||
| """ | ||
| defmacro __using__(opts) do | ||
| quote bind_quoted: [opts: opts] do | ||
| opts = Macro.expand(opts, __CALLER__) | ||
|
|
||
| if !Keyword.keyword?(opts) do | ||
| raise ArgumentError, "expected Backpex.Field options to be a keyword list, got: #{Macro.to_string(opts)}" | ||
| end | ||
|
Comment on lines
+243
to
+245
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 What changed. Before the PR, Shapes that break now. Each of these compiled fine before and now raises at compile time:
defmacro __using__(opts) do
quote bind_quoted: [opts: opts] do
use Backpex.Field, opts # opts is a variable here, not a literal list
end
endShapes that still work. A literal keyword list whose values are expressions is fine, because only the outer list shape is inspected. Why it matters anyway. The failure is a compile error with a misleading message. Someone with a shared base field module sees Fix. Only pop |
||
|
|
||
| {live_component, opts} = Keyword.pop(opts, :live_component) | ||
| live_component = live_component(live_component, __CALLER__) | ||
|
|
||
| quote bind_quoted: [opts: opts], unquote: true do | ||
| @config_schema opts[:config_schema] || [] | ||
|
|
||
| @before_compile Backpex.Field | ||
| @behaviour Backpex.Field | ||
|
|
||
| use BackpexWeb, :field | ||
| unquote(live_component) | ||
| use BackpexWeb, :field_helpers | ||
|
|
||
| @doc """ | ||
| Returns the schema of configurable options for this field. | ||
|
|
@@ -262,6 +282,30 @@ defmodule Backpex.Field do | |
| end | ||
| end | ||
|
|
||
| defp live_component(nil, _caller) do | ||
| quote do | ||
| use Phoenix.LiveComponent | ||
| end | ||
| end | ||
|
|
||
| defp live_component({module, entrypoint}, caller) when is_atom(entrypoint) do | ||
| module = Macro.expand(module, caller) | ||
|
|
||
| if !is_atom(module) do | ||
| raise ArgumentError, | ||
| "expected :live_component to contain a module, got: #{Macro.to_string(module)}" | ||
| end | ||
|
|
||
| quote do | ||
| use unquote(module), unquote(entrypoint) | ||
| end | ||
| end | ||
|
|
||
| defp live_component(live_component, _caller) do | ||
| raise ArgumentError, | ||
| "expected :live_component to be a {module, entrypoint} tuple, got: #{Macro.to_string(live_component)}" | ||
| end | ||
|
|
||
| defmacro __before_compile__(_env) do | ||
| quote generated: true do | ||
| import Ecto.Query | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| defmodule Backpex.WebHelpersTest do | ||
| use ExUnit.Case, async: false | ||
|
|
||
| @fixture Path.expand("../fixtures/web_helpers/extension_modules.fixture", __DIR__) | ||
|
|
||
| @fixture_modules [ | ||
| Backpex.WebHelpersTest.Helpers, | ||
| Backpex.WebHelpersTest.Web, | ||
| Backpex.WebHelpersTest.ItemActionBackpexFirst, | ||
| Backpex.WebHelpersTest.ItemActionWebFirst, | ||
| Backpex.WebHelpersTest.FilterBackpexFirst, | ||
| Backpex.WebHelpersTest.FilterWebFirst, | ||
| Backpex.WebHelpersTest.SelectFilterBackpexFirst, | ||
| Backpex.WebHelpersTest.SelectFilterWebFirst, | ||
| Backpex.WebHelpersTest.MetricBackpexFirst, | ||
| Backpex.WebHelpersTest.MetricWebFirst, | ||
| Backpex.WebHelpersTest.Field | ||
| ] | ||
|
|
||
| test "Backpex extensions compile with host web helpers without warnings" do | ||
| purge_fixture_modules() | ||
| on_exit(&purge_fixture_modules/0) | ||
|
|
||
| assert {:ok, modules, %{compile_warnings: [], runtime_warnings: []}} = | ||
| Kernel.ParallelCompiler.compile([@fixture], | ||
| max_concurrency: 1, | ||
| return_diagnostics: true | ||
| ) | ||
|
|
||
| assert Enum.sort(modules) == Enum.sort(@fixture_modules) | ||
| end | ||
|
|
||
| defp purge_fixture_modules do | ||
| Enum.each(@fixture_modules, fn module -> | ||
| :code.purge(module) | ||
| :code.delete(module) | ||
| end) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this fail on compile or on render? It should fail on compile.
(To be honest, I’m not a big fan of the syntax. Feels very much like a workaround. But I can’t think of anything better at the moment.)