Skip to content

Fix assign_async return docs#4236

Merged
SteffenDE merged 2 commits into
phoenixframework:mainfrom
preciz:fix-assign-async-docs
May 18, 2026
Merged

Fix assign_async return docs#4236
SteffenDE merged 2 commits into
phoenixframework:mainfrom
preciz:fix-assign-async-docs

Conversation

@preciz
Copy link
Copy Markdown
Contributor

@preciz preciz commented May 17, 2026

No description provided.

Comment thread lib/phoenix_live_view.ex
The function must return either a map or a keyword list with the assigns
to merge into the socket.
The function must return either `{:ok, assigns}` or `{:error, reason}`,
where `assigns` is a map with the keys passed to `assign_async/3`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A keyword list should work

Suggested change
where `assigns` is a map with the keys passed to `assign_async/3`.
where `assigns` is a map or keyword list with the keys passed to `assign_async/3`.

Copy link
Copy Markdown
Contributor Author

@preciz preciz May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work when keyword list is used:

{:ok, %{} = assigns} ->

Also just tested it manually again.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I guess the list check here is unnecessary then:

{:ok, {:ok, assigns}} when is_map(assigns) or is_list(assigns) ->
- if it never worked, we can adjust the documentation indeed

@preciz
Copy link
Copy Markdown
Contributor Author

preciz commented May 18, 2026

@SteffenDE what you think for current state of the PR?

Comment on lines +235 to +236
assert html =~ "data: 123"
refute html =~ "expected assign_async to return"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this assertion actually relied on the error message. A good example for a bad test 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it seems like.

@SteffenDE SteffenDE merged commit 51f3072 into phoenixframework:main May 18, 2026
8 checks passed
@SteffenDE
Copy link
Copy Markdown
Collaborator

🙌🏻

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.

3 participants