feat: maxFiles option

[OrJDev]

For instance:

 withoutMdwr: f
    .limits("max 2 files of 1GB each")
    .fileTypes(["image"])
    .middleware(async () => {
      return { testMetadata: "lol" };
    })
    .onUploadComplete(({ metadata, file }) => {
      console.log("uploaded with the following metadata:", metadata);
      metadata;
      // ^?

      console.log("files successfully uploaded:", file);
      file;
      // ^?
    }),

this will only allow 2 files to be passed from both client & server

closes https://github.com/pingdotgg/uploadthing/issues/44

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-uploadthing	❌ Failed (Inspect)			May 14, 2023 7:43pm

[juliusmarminge]

We should think of the relation between maxSize and maxFiles - is it 2 files with 64MB each or is 64MB the total size allowed?

[OrJDev]

We should think of the relation between maxSize and maxFiles - is it 2 files with 64MB each or is 64MB the total size allowed?

That would be maximum of 2 files, and a limit of 64MB total, it makes more sense to define this way as you want to choose both maximum urls to save in your db & maximum size total for a single product for instance (it doesn't changes the current functionality, it just slices up the files to the maxFile if specified so the user can choose to only upload one for instance)

[markflorkowski]

May be more clear as maxFileCount(count: number), possibly fileCount(count: number, type: "exact" | "max")

(if we want the restrictions to all apply to each file individually)

If we want to make that a total for all files, there will be required change to the infra side to respect that.

[OrJDev]

May be more clear as maxFileCount(count: number), possibly fileCount(count: number, type: "exact" | "max")

(if we want the restrictions to all apply to each file individually)

If we want to make that a total for all files, there will be required change to the infra side to respect that.

Implemented that, but you will also need to change the infra api:

https://github.com/pingdotgg/uploadthing/pull/47/commits/ae447822ae2e8a40aad303a8c3fc12c58f379fef#diff-9cb45f5c348aa23d75574803fb7b59cecfeaf242247187e9a563148331fb9f4bR231

now i'm passing all of the info but make sure to also handle it on your end

[markflorkowski]

I think I am still of the opinion that fileSize should apply to each individual file. Possibly renaming to maxFileSize(), though this would be breaking.

@t3dotgg -- opinions?

[juliusmarminge]

To me its still unclear what

f
    .maxSize("64MB")
    .fileCount(2, "max")

will do.

On one hand the missing File in maxSize indicates that its the entire payload's size, but its not the best wording combined with this new option.

I think we can make something like this typesafe:

f
  .limits("max 2 files of 16MB each")
  // or
  .limits("max 2 files of 16MB total")

Might be too many words, could be split to many

f
  .maxFileCount(2)
  .maxFileSize("32MB")
  .maxPayloadSize("48MB")

[OrJDev]

To me its still unclear what

f
    .maxSize("64MB")
    .fileCount(2, "max")

will do.

On one hand the missing File in maxSize indicates that its the entire payload's size, but its not the best wording combined with this new option.

I think we can make something like this typesafe:

f
  .limits("max 2 files of 16MB each")
  // or
  .limits("max 2 files of 16MB total")

Might be too many words, could be split to many

f
  .maxFileCount(2)
  .maxFileSize("32MB")
  .maxPayloadSize("48MB")

I really do like the api julius suggested (the first 1), tho its a lot of words this is very clear to the user what is going to happen and its very easy to make it type safe, will update this pr 🤶

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 7fffe29f3083b395ae35c2bd43f872f768aec961

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
uploadthing	Patch
@uploadthing/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[OrJDev]

Updated the pr, it now looks like:

// FileRouter for your app, can contain multiple FileRoutes
export const ourFileRouter = {
  // Define as many FileRoutes as you like, each with a unique routeSlug
  imageUploader: f
    // Set permissions and file types for this FileRoute
    .fileTypes(["image", "video"])
    // only allow maximum of 2 files to be passed in a single request,
    // each file must be less than 1GB
    .limits("max 2 files of 1GB each")
    .middleware(async (req, res) => {
      // This code runs on your server before upload
      const user = await auth(req, res);

      // If you throw, the user will not be able to upload
      if (!user) throw new Error("Unauthorized");

      // Whatever is returned here is accessible in onUploadComplete as `metadata`
      return { userId: user.id };
    })
    .onUploadComplete(async ({ metadata, file }) => {
      // This code RUNS ON YOUR SERVER after upload
      console.log("Upload complete for userId:", metadata.userId);

      console.log("file url", file.url);
    }),
} satisfies FileRouter;

export type OurFileRouter = typeof ourFileRouter;

We pass all of the info to the infra api like:

{ 
maxFiles: uploadable._def.maxFiles,
            minFiles: uploadable._def.minFiles,
            exactFiles: uploadable._def.exactFiles,
            maxFileSizeKind: uploadable._def.maxFileSizeKind
}

so make sure it is handled there, sadly i can't help there as i don't have access to this code so you guys have to do it

https://github.com/pingdotgg/uploadthing/pull/47/files#diff-9cb45f5c348aa23d75574803fb7b59cecfeaf242247187e9a563148331fb9f4bR237

[OrJDev]

Not sure why deployment is failing, can anyone with access tell me the error?

[l-mbert]

I personally don't like the "max 2 files of 1gb each" part. I assume it has type-safety but this would be harder to create and/or reuse by other functions. I would prefer something like this:

...
.limits({
  count: 2,
  size: "1gb",
  kind: "each"
})
...

Then I could create a function getUploadthingLimits() which returns something I could better use somewhere else than a string which needs to be parsed.

[OrJDev]

I personally don't like the "max 2 files of 1gb each" part. I assume it has type-safety but this would be harder to create and/or reuse by other functions. I would prefer something like this:

...
.limits({
  count: 2,
  size: "1gb",
  kind: "each"
})
...

Then I could create a function getUploadthingLimits() which returns something I could better use somewhere else than a string which needs to be parsed.

It is type-safe but ofc we can also make it accept an object like you offered not just string, can be both.

Type is declared here: https://github.com/pingdotgg/uploadthing/pull/47/files#diff-ba483f7b17e896d2a3ef45c2c2719d9f208743ea2927875fd20e53cddd9dc383R67

[t3dotgg]

Been thinking about this too much. I was close then @l-mbert's proposal made me rethink everything. Here's where I'm at, ts playground linked below

const routeOne = f(["image"])
  .middleware(() => ({ id: "none" }))
  .onSuccess((data) => console.log("file", data));

const routeTwo = f(["image", "video"])
  .middleware(() => ({ id: "none" }))
  .onSuccess((data) => console.log("file", data));

const routeThree = f({ image: { maxSize: "2MB", limit: 1 } })
  .middleware(() => ({ id: "none" }))
  .onSuccess((data) => console.log("file", data));

const routeFour = f({
  image: { maxSize: "2MB", limit: 2 },
  video: { maxSize: "200MB", limit: 1 },
})
  .middleware(() => ({ id: "none" }))
  .onSuccess((data) => console.log("file", data));

https://tsplay.dev/mLv2vw

[t3dotgg]

Inside a router, refined a bit:

const uploadThingRouter = {
  profilePicture: f(["image"])
    .middleware((req) => auth(req))
    .onUploadComplete((data) => console.log("file", data)),

  messageAttachment: f(["image", "video"])
    .middleware((req) => auth(req))
    .onUploadComplete((data) => console.log("file", data)),

  strictImageAttachment: f({ image: { maxFileSize: "2MB", limit: 1 } })
    .middleware((req) => auth(req))
    .onUploadComplete((data) => console.log("file", data)),

  mediaPost: f({
    image: { maxFileSize: "2MB", limit: 4 },
    video: { maxFileSize: "200MB", limit: 1 },
  })
    .middleware((req) => auth(req))
    .onUploadComplete((data) => console.log("file", data)),
};

[l-mbert]

Okay, after a bit of thinking my comment earlier (which I deleted) wasn't so stupid as I thought after sending it.

I like this solution @t3dotgg. But @OrJDev solution also added the possibility to say if the limit is for "each" of the files or for the "total" of all files. Maybe could add something like this to preserve that behaviour:

...
mediaPost: f({
    image: { maxFileSize: "2MB", limit: 4, fileRestrictionType: "total" },
    video: { maxFileSize: "200MB", limit: 2, fileRestrictionType: "each" },
  })
    .middleware((req) => auth(req))
    .onUploadComplete((data) => console.log("file", data)),
...

So when calling the "mediaPost" endpoint we can upload 4 files of a total size of 2mb and 2 videos of 200mb each.

[markflorkowski]

I think part of the proposal that Theo is making is that "total payload size" is not something that the file router should be handling. It's not a super common use case, and making the API more complicated/confusing to enable it does not seem worthwhile at this point.

Correct me if I am wrong @t3dotgg

[t3dotgg]

@markflorkowski's comment is correct. I don't think total upload size is a particularly common use-case. If anyone has compelling examples please feel free to share

[l-mbert]

I agree that it's not super common but if it applies it would be weird to implement.

An example I'm currently facing is this: I have a post where the user can upload a gallery of images. They can upload up to 4 images but I don't want the post to have over 2MB of images. I could apply the restriction of 500KB to each image but then the user could not upload a high quality picture and 3 low quality ones. Or even just one 2MB big hq picture.

If you do decide against it I'm totally fine with it. I can understand why.

[t3dotgg]

@l-mbert it's really unintuitive to me as to why you'd want to limit the whole gallery instead of individual images. Why would you want a gallery to have a lower "target" size than an individual post? This sounds like really bad UX

If you have an example out in the wild, I'll reconsider, but for now I'm closing this in favor of the configuration direction I shared above

Ty for the contributions and pushing us to think more on this