Skip to content

Drop phoenix_html_helpers and make postgrex optional - #2506

Open
Flo0807 wants to merge 5 commits into
developfrom
refactor/drop-one-use-deps
Open

Flo0807 wants to merge 5 commits into
developfrom
refactor/drop-one-use-deps

Conversation

@Flo0807

@Flo0807 Flo0807 commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

What was wrong

Backpex forced two packages on every app and used each of them in one place:

  • phoenix_html_helpers: lib/backpex/fields/has_many_through.ex imported PhoenixHTMLHelpers.Form.hidden_inputs_for/1 for the hidden inputs (the pivot record's primary key) in the edit modal of each pivot row.
  • postgrex: lib/backpex/item_actions/delete.ex matched %Postgrex.Error{postgres: %{code: :foreign_key_violation}} in two heads of error_message/3 to add "The item is used elsewhere." / "The items are used elsewhere." to the flash.

What changed

  • Backpex.Fields.HasManyThrough renders the hidden inputs with a plain HEEx loop over e.hidden, with Phoenix.HTML.Form.input_id/2 and input_name/2. The markup is the same as before: <input id="parent_pivots_0_id" name="parent[pivots][0][id]" type="hidden" value="11">. phoenix_html_helpers is removed from mix.exs, mix.lock and demo/mix.lock (the demo only had it through Backpex).
  • Backpex.ItemActions.Delete matches the error as %{__struct__: Postgrex.Error, postgres: %{code: :foreign_key_violation}}, which needs no loaded module, and postgrex is optional: true. The lock files keep postgrex: Backpex's own project still fetches an optional dependency, and the demo lists it itself.
  • Docs: new guides/upgrading/v0.25.md (apps list postgrex themselves, and phoenix_html_helpers if their own code calls it), and the installation guide names PostgreSQL and postgrex in the prerequisites.

Label breaking-change: an app that got one of the packages only through Backpex has to list it now.

How it was tested

The first commit only adds tests that pin the behaviour. They pass with lib/, mix.exs and mix.lock of develop (16 passed in the three files) and with the change:

  • test/fields/has_many_through_test.exs: "renders the hidden inputs of each pivot row" compares the hidden inputs as written in the HTML, for two existing pivot rows and a new one.
  • integration_test/fields/has_many_through_test.exs: "keeps its pivot record when the form is saved" saves the edit form of a user with an address and checks that the pivot record keeps its id.
  • integration_test/item_actions/delete_test.exs: "an item that other records reference" deletes addresses that a user references, one from the show view and two selected on the index, and checks both flash messages. There was no test for these messages before. The adapter deletes with Repo.delete_all/1, so this is the Postgrex.Error path.

Both HasManyThrough tests fail when the loop renders no inputs (checked by replacing e.hidden with []).

Commands (all exit 0):

  • MIX_ENV=test mix lint: 2335 mods/funs, found no issues., 1367 passed (167 doctests, 1200 tests), 60 excluded
  • mix deps.unlock --check-unused
  • mix docs --warnings-as-errors
  • mix gettext.extract --check-up-to-date
  • INTEGRATION_TESTS=false mix test: 1041 passed (167 doctests, 874 tests)
  • Without postgrex: with the postgrex line removed from mix.exs in a separate build path, INTEGRATION_TESTS=false mix compile --force --warnings-as-errors and INTEGRATION_TESTS=false mix test passed (1041 passed), and Code.ensure_loaded?(Postgrex.Error) was false while Backpex.ItemActions.Delete loaded.
  • demo/: mix deps.unlock --check-unused failed with :phoenix_html_helpers before mix deps.unlock --unused and passes after it.

Surfaces

  • Views: the form for :edit (HasManyThrough modal), item action Delete on :show and :index (bulk).
  • Built-ins: Backpex.Fields.HasManyThrough only; BelongsTo, HasMany and InlineCRUD don't use the helper.
  • Adapter boundary: untouched; the item action still only inspects the error the adapter raises.
  • Texts: no string changed, backpex.pot is up to date.
  • Accessibility, themes, mobile: the rendered markup is unchanged, so nothing to check.
  • Fixtures: the existing AddressLive and UserLive fixtures cover the new tests.
  • User docs: installation guide and upgrade guide. usage-rules.md and plugin/skills/ don't mention the two packages.

For the reviewer

  • hidden_inputs_for/1 also handled list values (name[], indexed ids). The loop does not: the form of a pivot row comes from the changeset, where hidden holds the primary key fields of the pivot schema. A pivot schema with an array-typed primary key would render differently; I did not test that case.
  • demo/mix.lock was updated with mix deps.unlock --unused on the host, not in the demo container. The demo was not compiled or started, and bun run lint of the demo did not run. demo/lib/demo_web/item_actions/user_soft_delete.ex still matches %Postgrex.Error{}, which works because the demo lists postgrex itself.
  • No browser test ran; the change needs none.

@Flo0807 Flo0807 added the breaking-change A breaking change label Nov 11, 2026
The hidden inputs of the HasManyThrough pivot rows and the "used
elsewhere" message of the delete item action each depend on a package
Backpex uses nowhere else. These tests pin both behaviours before the
packages are dropped.
Backpex used each package once: phoenix_html_helpers for the hidden
inputs of the HasManyThrough pivot rows, postgrex for the struct of a
foreign key violation in the delete item action. The inputs are plain
HEEx now and the error is matched as a map, so apps no longer get both
packages through Backpex.
@Flo0807
Flo0807 force-pushed the refactor/drop-one-use-deps branch from 1c60d38 to 28b189a Compare November 11, 2026 17:08
@codecov

codecov Bot commented Nov 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Flo0807 Flo0807 self-assigned this Nov 13, 2026
@Flo0807
Flo0807 requested a review from pehbehbeh November 13, 2026 13:32

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change A breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant