phoenixframework
Entries compounding/not clearing out when using LiveUpload from a hook
Environment
- Elixir version (elixir -v): 1.18.1
- Phoenix version (mix deps): 1.17.18
- Phoenix LiveView version (mix deps): 1.0.1
- Operating system: OSX
- Browsers you attempted to reproduce this bug on (the more the merrier): Chrome, Firefox
- Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes
Actual behavior
I'm building a queued uploader, where I'll only upload one file at a time. I stumbled into some weirdness, though. If I queue up 20 files, the first file gets consumed and removed from entries, but is readded again on the next upload. I first checked dispatchUploads on the LV JS side, and that seems correct — it only sends one file at a time. I then checked maybe_update_uploads in Phoenix.LiveView.Channel and here it seems to compound.
The first upload:
[debug] => maybe_update_uploads -- %{
"phx-GBY54CJK-JzSgAFo" => [
%{
"last_modified" => 1704894902000,
"name" => "1",
"path" => "transformer",
"ref" => "0",
"relative_path" => "",
"size" => 153600,
"type" => ""
}
]
}
The second upload:
[debug] => maybe_update_uploads -- %{
"phx-GBY54CJK-JzSgAFo" => [
%{
"last_modified" => 1704894902000,
"name" => "1",
"path" => "transformer",
"ref" => "0",
"relative_path" => "",
"size" => 153600,
"type" => ""
},
%{
"last_modified" => 1704894902000,
"name" => "2",
"path" => "transformer",
"ref" => "1",
"relative_path" => "",
"size" => 153600,
"type" => ""
}
]
}
etc.
This means that the entries remaining in the uploaders config. Maybe they're not cleared out somewhere when using this.upload from a hook? I couldn't find out more, unfortunately..
Single file app here:
Application.put_env(:sample, Example.Endpoint,
http: [ip: {127, 0, 0, 1}, port: 5001],
server: true,
live_view: [signing_salt: "aaaaaaaa"],
secret_key_base: String.duplicate("a", 64),
pubsub_server: Example.PubSub
)
require Logger
Mix.install([
{:plug_cowboy, "~> 2.5"},
{:jason, "~> 1.2"},
{:phoenix, "~> 1.7.18"},
{:phoenix_live_view, "~> 1.0.1"}
])
defmodule Example.ErrorView do
def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end
defmodule Example.CoreComponents do
use Phoenix.Component
attr(:for, :any, required: true, doc: "the datastructure for the form")
attr(:as, :any, default: nil, doc: "the server side parameter to collect all input under")
attr(:rest, :global,
include: ~w(autocomplete name rel action enctype method novalidate target multipart),
doc: "the arbitrary HTML attributes to apply to the form tag"
)
slot(:inner_block, required: true)
slot(:actions, doc: "the slot for form actions, such as a submit button")
def simple_form(assigns) do
~H"""
<.form :let={f} for={@for} as={@as} {@rest}>
<div>
<%= render_slot(@inner_block, f) %>
<div :for={action <- @actions}>
<%= render_slot(action, f) %>
</div>
</div>
</.form>
"""
end
end
defmodule Example.UploadLive do
use Phoenix.LiveView, layout: {__MODULE__, :live}
import Example.CoreComponents
def mount(_params, _session, socket) do
socket =
socket
|> allow_upload(:files,
accept: :any,
max_entries: 1500,
max_file_size: 10_000_000_000,
auto_upload: true,
progress: &handle_progress/3
)
|> assign(:form, to_form(%{}))
{:ok, socket}
end
defp phx_vsn, do: Application.spec(:phoenix, :vsn)
defp lv_vsn, do: Application.spec(:phoenix_live_view, :vsn)
def render("live.html", assigns) do
~H"""
<script src={"https://cdn.jsdelivr.net/npm/phoenix@#{phx_vsn()}/priv/static/phoenix.min.js"}></script>
<script src={"https://cdn.jsdelivr.net/npm/phoenix_live_view@#{lv_vsn()}/priv/static/phoenix_live_view.min.js"}></script>
<script>
const QueuedUploaderHook = {
async mounted() {
const maxConcurrency = 1
let filesRemaining = []
this.el.addEventListener('input', async (event) => {
event.preventDefault()
// try to wait a tick — without this delay, nothing happens,
// it readies the first 3 files but doesn't upload them
setTimeout(() => {
if (event.target instanceof HTMLInputElement) {
const files_html = event.target.files
if (files_html) {
filesRemaining = Array.from(files_html)
const firstFiles = filesRemaining.slice(0, maxConcurrency)
console.log('firstFiles', firstFiles)
this.upload('files', firstFiles)
console.log('filesRemaining', filesRemaining)
filesRemaining.splice(0, maxConcurrency)
console.log('filesRemaining after splice', filesRemaining)
}
}
}, 0)
})
this.handleEvent('upload_send_next_file', () => {
console.log('Uploading more files! Remainder:', filesRemaining)
if (filesRemaining.length > 0) {
console.log('got MORE!')
const nextFile = filesRemaining.shift()
if (nextFile != undefined) {
console.log('Uploading file: ', nextFile.name)
this.upload('files', [nextFile])
}
} else {
console.log('Done uploading, noop!')
}
})
}
};
let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket, {hooks: {QueuedUploaderHook}});
liveSocket.connect();
</script>
<%= @inner_content %>
"""
end
def render(assigns) do
~H"""
<main>
<h1>Uploader reproduction</h1>
<!-- moved outside of form like in the upload by hook example from mcrumm,
prevents extra validate event -->
<input
id="fileinput"
type="file"
multiple
phx-hook="QueuedUploaderHook"
disabled={file_picker_disabled?(@uploads)}
/>
<.simple_form for={@form} phx-submit="save" phx-change="validate">
<section>
<.live_file_input upload={@uploads.files} style="display: none;" />
<h2 :if={length(@uploads.files.entries) > 0}>Currently uploading files</h2>
<div>
<table>
<!-- head -->
<thead>
<tr>
<th>File Name</th>
<th>Progress</th>
<th>Cancel</th>
<th>Errors</th>
</tr>
</thead>
<tbody>
<%= for entry <- uploads_in_progress(@uploads) do %>
<tr>
<td><%= entry.client_name %></td>
<td>
<progress value={entry.progress} max="100">
<%= entry.progress %>%
</progress>
</td>
<td>
<button
type="button"
phx-click="cancel-upload"
phx-value-ref={entry.ref}
aria-label="cancel"
>
<span>×</span>
</button>
</td>
<td>
<%= for err <- upload_errors(@uploads.files, entry) do %>
<p style="color: red;"><%= error_to_string(err) %></p>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
</div>
<%!-- Phoenix.Component.upload_errors/1 returns a list of error atoms --%>
<%= for err <- upload_errors(@uploads.files) do %>
<p style="text-red"><%= error_to_string(err) %></p>
<% end %>
</section>
</.simple_form>
</main>
"""
end
def handle_progress(:files, entry, socket) do
if entry.done? do
case consume_uploaded_entry(socket, entry, fn _meta -> {:ok, "ok"} end) do
"ok" ->
Logger.debug("=> consumed")
{:noreply, push_event(socket, "upload_send_next_file", %{})}
end
else
{:noreply, socket}
end
end
def handle_event("validate", _params, socket) do
{:noreply, socket}
end
def handle_event("cancel-upload", %{"ref" => ref}, socket) do
{:noreply, cancel_upload(socket, :files, ref)}
end
def handle_event("save", _params, socket) do
{:noreply, socket}
end
def error_to_string(:too_large), do: "Too large"
def error_to_string(:not_accepted), do: "You have selected an unacceptable file type"
def error_to_string(:s3_error), do: "Error on writing to cloudflare"
def error_to_string(unknown) do
IO.inspect(unknown, label: "Unknown error")
"unknown error"
end
## Helpers
defp file_picker_disabled?(uploads) do
Enum.any?(uploads.files.entries, fn e -> !e.done? end)
end
defp uploads_in_progress(uploads) do
uploads.files.entries
end
end
defmodule Example.Router do
use Phoenix.Router
import Phoenix.LiveView.Router
pipeline :browser do
plug(:accepts, ["html"])
end
scope "/", Example do
pipe_through(:browser)
live("/", UploadLive, :index)
end
end
defmodule Example.Endpoint do
use Phoenix.Endpoint, otp_app: :sample
socket("/live", Phoenix.LiveView.Socket)
plug(Example.Router)
end
{:ok, _} =
Supervisor.start_link(
[Example.Endpoint, {Phoenix.PubSub, [name: Example.PubSub, adapter: Phoenix.PubSub.PG2]}],
strategy: :one_for_one
)
Process.sleep(:infinity)
Expected behavior
Entries are cleared out when consumed and there isn't any compounding of upload entries when uploading through a hook.
I've checked most of the JS code to locate where the extra entries are sent without much luck. However I can inspect the WS connection and it looks to be in the validate events.
The first validate event looks OK:
[
"4",
"14",
"lv:phx-GBZDkJIZVqWmUwCG",
"event",
{
"type": "form",
"event": "validate",
"value": "_target=files",
"uploads": {
"phx-GBZDkKKTiWKJPwDG": [
{
"path": "files",
"ref": "0",
"last_modified": 1704894902000,
"name": "2",
"relative_path": "",
"type": "",
"size": 153600
}
]
}
}
]
then the next validate event:
[
"4",
"25",
"lv:phx-GBZDkJIZVqWmUwCG",
"event",
{
"type": "form",
"event": "validate",
"value": "_target=files",
"uploads": {
"phx-GBZDkKKTiWKJPwDG": [
{
"path": "files",
"ref": "0",
"last_modified": 1704894902000,
"name": "2",
"relative_path": "",
"type": "",
"size": 153600
},
{
"path": "files",
"ref": "1",
"last_modified": 1704894902000,
"name": "3",
"relative_path": "",
"type": "",
"size": 153600
}
]
}
}
]
still carries the previous file
The third event shifts out the first file, but now carries the second and third file:
[
"4",
"36",
"lv:phx-GBZDkJIZVqWmUwCG",
"event",
{
"type": "form",
"event": "validate_catalog",
"value": "_target=transformer",
"uploads": {
"phx-GBZDkKKTiWKJPwDG": [
{
"path": "transformer",
"ref": "1",
"last_modified": 1704894902000,
"name": "3",
"relative_path": "",
"type": "",
"size": 153600
},
{
"path": "transformer",
"ref": "2",
"last_modified": 1704894902000,
"name": "4",
"relative_path": "",
"type": "",
"size": 153600
}
]
}
}
]
etc
So:
If I add a setTimeout to the upload_send_next_file event it works. The entries are cleared out.
this.handleEvent('upload_send_next_file', () => {
console.log('Uploading more files! Remainder:', filesRemaining)
// waiting a tick here to allow LV to reset the files(?)
setTimeout(() => {
if (filesRemaining.length > 0) {
console.log('got MORE!')
const nextFile = filesRemaining.shift()
if (nextFile != undefined) {
console.log('Uploading file: ', nextFile.name)
this.upload('files', [nextFile])
}
} else {
console.log('Done uploading, noop!')
}
}, 0)
})
Maybe this is to be expected?
Sorry for the noise / 🦆 —— last note: It also works with this.upload wrapped in a window.requestAnimationFrame, which I see is used in other places in the codebase. I could PR this change in the this.upload function, if it seems right? I'm a little unsure of where this function resides though... 😅
@tmjoen I didn't look into this in detail yet, but it looks very similar to https://github.com/phoenixframework/phoenix_live_view/blob/main/test/e2e/support/issues/issue_2965.ex; do you know what's the difference?
@SteffenDE Thanks for looking! In the issue_2965 file it initially waits for the "upload_scrub_list" which will delay the time between the input event and this.upload in the same way as requestAnimationFrame I guess? Also maybe it's different with the noop writer?
@tmjoen this is because currently events are dispatched before the push is acknowledged to the caller. We can fix this in LV by dispatching events in the next microtick.
diff --git a/assets/js/phoenix_live_view/view.js b/assets/js/phoenix_live_view/view.js
index 6b9fe4ce..c01986aa 100644
--- a/assets/js/phoenix_live_view/view.js
+++ b/assets/js/phoenix_live_view/view.js
@@ -669,7 +669,9 @@ export default class View {
})
}
- this.liveSocket.dispatchEvents(events)
+ // dispatch events in the next microtick to allow file progress to be tracked
+ // before hook event handlers are invoked (#3610)
+ window.requestAnimationFrame(() => { this.liveSocket.dispatchEvents(events) })
if(phxChildrenAdded){ this.joinNewChildren() }
}
@chrismccord the pushFileProgress callback is only called when the pushWithReply promise resolves, but the (hook) events are dispatched earlier: https://github.com/phoenixframework/phoenix_live_view/blob/38be0a11eb8f509ac6995c5557d782327371820c/assets/js/phoenix_live_view/upload_entry.js#L61-L64
Let's see what Chris thinks :)