fix: run onUploadBegin (#952)

Co-authored-by: Mark R. Florkowski <[email protected]>

Author: juliusmarminge

.changeset/spotty-spoons-chew.md

@@ -0,0 +1,5 @@
+---
+"uploadthing": patch
+---
+
+fix: run `onUploadBegin`

docs/src/app/(docs)/api-reference/client/page.mdx

@@ -55,9 +55,9 @@ is an options object:
   <Property name="signal" type="AbortSignal" since="6.7">
     An abort signal to abort the upload.
   </Property>
-  <Property name="onUploadBegin" type="({ file }) => void" since="5.4">
+  <Property name="onUploadBegin" type="({ file: string }) => void" since="5.4">
     Callback function called after the presigned URLs have been retrieved, just before 
-    the files are uploaded to the storage provider.
+    the file is uploaded. Called once per file.
   </Property>
   <Property
     name="onUploadProgress"

docs/src/app/(docs)/api-reference/react/page.mdx

@@ -264,9 +264,9 @@ export const OurUploadButton = () => (
     returned are the files that will be uploaded, meaning you can use this to
     e.g. rename or resize the files.
   </Property>
-  <Property name="onUploadBegin" type="function" since="5.4">
-    Callback function called after the presigned URLs have been retrieved, just
-    before the files are uploaded to the storage provider.
+  <Property name="onUploadBegin" type="({ file: string }) => void" since="5.4">
+    Callback function called after the presigned URLs have been retrieved, just before 
+    the file is uploaded. Called once per file.
   </Property>
   <Property name="disabled" type="boolean" since="6.7" defaultValue="false">
     Disables the button.
@@ -405,9 +405,9 @@ export const OurUploadDropzone = () => (
     returned are the files that will be uploaded, meaning you can use this to
     e.g. rename or resize the files.
   </Property>
-  <Property name="onUploadBegin" type="function" since="5.4">
-    Callback function called after the presigned URLs have been retrieved, just
-    before the files are uploaded to the storage provider.
+  <Property name="onUploadBegin" type="({ file: string }) => void" since="5.4">
+    Callback function called after the presigned URLs have been retrieved, just before 
+    the file is uploaded. Called once per file.
   </Property>
   <Property name="disabled" type="boolean" since="6.7" defaultValue="false">
     Disables the button.
@@ -512,7 +512,7 @@ is an options object:
     Callback function that gets continuously called as the file is uploaded to
     the storage provider.
   </Property>
-  <Property name="onUploadBegin" type="function" since="5.4">
+  <Property name="onUploadBegin" type="({ file: string }) => void" since="5.4">
     Callback function called after the presigned URLs have been retrieved, just
     before the files are uploaded to the storage provider.
   </Property>
@@ -565,8 +565,8 @@ export function MultiUploader() {
     onUploadError: () => {
       alert("error occurred while uploading");
     },
-    onUploadBegin: () => {
-      alert("upload has begun");
+    onUploadBegin: ({ file }) => {
+      console.log("upload has begun for", file);
     },
   });
 

packages/react/test/upload-button.test.tsx

@@ -31,7 +31,13 @@ const testRouter = {
   pdf: f({ "application/pdf": {} }).onUploadComplete(noop),
   multi: f({ image: { maxFileCount: 4 } }).onUploadComplete(noop),
 };
-const routeHandler = createRouteHandler({ router: testRouter });
+const routeHandler = createRouteHandler({
+  router: testRouter,
+  config: {
+    token:
+      "eyJhcHBJZCI6ImFwcC0xIiwiYXBpS2V5Ijoic2tfZm9vIiwicmVnaW9ucyI6WyJmcmExIl19",
+  },
+});
 const UploadButton = generateUploadButton<typeof testRouter>();
 
 const utGet = vi.fn<(req: Request) => void>();
@@ -44,13 +50,16 @@ const server = setupServer(
     return routeHandler(request);
   }),
   http.post("/api/uploadthing", async ({ request }) => {
-    const body = await request.json();
+    const body = await request.clone().json();
     utPost({ request, body });
-    return HttpResponse.json([
-      // empty array, we're not testing the upload endpoint here
-      // we have other tests for that...
-    ]);
+    return routeHandler(request);
   }),
+  http.all<{ key: string }>(
+    "https://fra1.ingest.uploadthing.com/:key",
+    ({ request, params }) => {
+      return HttpResponse.json({ url: "https://utfs.io/f/" + params.key });
+    },
+  ),
 );
 
 beforeAll(() => server.listen());
@@ -203,6 +212,25 @@ describe("UploadButton - lifecycle hooks", () => {
       );
     });
   });
+
+  it("onUploadBegin runs before uploading", async () => {
+    const onUploadBegin = vi.fn();
+    const utils = render(
+      <UploadButton endpoint="multi" onUploadBegin={onUploadBegin} />,
+    );
+    await waitFor(() => {
+      expect(utils.getByText("Choose File(s)")).toBeInTheDocument();
+    });
+
+    fireEvent.change(utils.getByLabelText("Choose File(s)"), {
+      target: { files: [new File([""], "foo.png"), new File([""], "bar.png")] },
+    });
+    await waitFor(() => {
+      expect(onUploadBegin).toHaveBeenCalledTimes(2);
+    });
+    expect(onUploadBegin).toHaveBeenCalledWith("foo.png");
+    expect(onUploadBegin).toHaveBeenCalledWith("bar.png");
+  });
 });
 
 describe("UploadButton - Theming", () => {

packages/uploadthing/src/internal/upload.browser.ts

@@ -146,22 +146,32 @@ export const uploadFilesInternal = <
       Micro.forEach(
         presigneds,
         (presigned, i) =>
-          uploadFile<TRouter, TEndpoint, TServerOutput>(
-            opts.files[i],
-            presigned,
-            {
-              onUploadProgress: (ev) => {
-                totalLoaded += ev.delta;
-                opts.onUploadProgress?.({
-                  file: opts.files[i],
-                  progress: Math.round((ev.loaded / opts.files[i].size) * 100),
-                  loaded: ev.loaded,
-                  delta: ev.delta,
-                  totalLoaded,
-                  totalProgress: Math.round((totalLoaded / totalSize) * 100),
-                });
-              },
-            },
+          Micro.flatMap(
+            Micro.sync(() =>
+              opts.onUploadBegin?.({ file: opts.files[i].name }),
+            ),
+            () =>
+              uploadFile<TRouter, TEndpoint, TServerOutput>(
+                opts.files[i],
+                presigned,
+                {
+                  onUploadProgress: (ev) => {
+                    totalLoaded += ev.delta;
+                    opts.onUploadProgress?.({
+                      file: opts.files[i],
+                      progress: Math.round(
+                        (ev.loaded / opts.files[i].size) * 100,
+                      ),
+                      loaded: ev.loaded,
+                      delta: ev.delta,
+                      totalLoaded,
+                      totalProgress: Math.round(
+                        (totalLoaded / totalSize) * 100,
+                      ),
+                    });
+                  },
+                },
+              ),
           ),
         { concurrency: 6 },
       ),

packages/uploadthing/test/client.test.ts

@@ -2,7 +2,7 @@
 
 import type { AddressInfo } from "node:net";
 import express from "express";
-import { describe, expect, expectTypeOf, it as rawIt } from "vitest";
+import { describe, expect, expectTypeOf, it as rawIt, vi } from "vitest";
 
 import { genUploader } from "../src/client";
 import { createRouteHandler, createUploadthing } from "../src/express";
@@ -34,6 +34,13 @@ export const setupUTServer = async () => {
       })
       .onUploadError(onErrorMock)
       .onUploadComplete(uploadCompleteMock),
+    multi: f({ text: { maxFileSize: "16MB", maxFileCount: 2 } })
+      .middleware((opts) => {
+        middlewareMock(opts);
+        return {};
+      })
+      .onUploadError(onErrorMock)
+      .onUploadComplete(uploadCompleteMock),
     withServerData: f(
       { text: { maxFileSize: "4MB" } },
       { awaitServerData: true },
@@ -264,6 +271,21 @@ describe("uploadFiles", () => {
     await close();
   });
 
+  it("handles too many files errors", async ({ db }) => {
+    const { uploadFiles, close } = await setupUTServer();
+
+    const file1 = new File(["foo"], "foo.txt", { type: "text/plain" });
+    const file2 = new File(["bar"], "bar.txt", { type: "text/plain" });
+
+    await expect(
+      uploadFiles("foo", { files: [file1, file2] }),
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `[UploadThingError: Invalid config: FileCountMismatch]`,
+    );
+
+    await close();
+  });
+
   it("handles invalid file type errors", async ({ db }) => {
     const { uploadFiles, close } = await setupUTServer();
 
@@ -277,4 +299,38 @@ describe("uploadFiles", () => {
 
     await close();
   });
+
+  it("runs onUploadBegin before uploading (single file)", async () => {
+    const { uploadFiles, close } = await setupUTServer();
+
+    const file = new File(["foo"], "foo.txt", { type: "text/plain" });
+    const onUploadBegin = vi.fn();
+
+    await uploadFiles("foo", {
+      files: [file],
+      onUploadBegin,
+    });
+
+    expect(onUploadBegin).toHaveBeenCalledWith({ file: "foo.txt" });
+
+    await close();
+  });
+
+  it("runs onUploadBegin before uploading (multi file)", async () => {
+    const { uploadFiles, close } = await setupUTServer();
+
+    const file1 = new File(["foo"], "foo.txt", { type: "text/plain" });
+    const file2 = new File(["bar"], "bar.txt", { type: "text/plain" });
+    const onUploadBegin = vi.fn();
+
+    await uploadFiles("multi", {
+      files: [file1, file2],
+      onUploadBegin,
+    });
+
+    expect(onUploadBegin).toHaveBeenCalledWith({ file: "foo.txt" });
+    expect(onUploadBegin).toHaveBeenCalledWith({ file: "bar.txt" });
+
+    await close();
+  });
 });