uppy icon indicating copy to clipboard operation
uppy copied to clipboard

@uppy/transloadit: always create one assembly instead of determining per file

Open Murderlon opened this issue 8 months ago • 9 comments

Initial checklist

  • [X] I understand this is a feature request and questions should be posted in the Community Forum
  • [X] I searched issues and couldn’t find anything (or linked relevant results below)

Problem

Not too long ago, we deprecated most Transloadit options in favor of assemblyOptions. You now pass assemblyOptions as an object or as a function which is called for every file.

The problem is that these options never change. In production people need to generate a signature on the server and thus doing the following is very common:

uppy.use(Transloadit, {
    async assemblyOptions(file) {
        const res = await fetch('/transloadit-params');
        return response.json();
    },
});

This now happens for every file, which is unneeded. So why do we call this per file? This goes back at least 6 years and I couldn't easily find what the PR was that introduced it.

Let's look at a test to tell us a bit more:

https://github.com/transloadit/uppy/blob/37d0a03390b4e704949dbe5f2bfc9e116e3cba1c/packages/%40uppy/transloadit/src/AssemblyOptions.test.js#L20-L49

What this test means

  • We call assemblyOptions per file in order to return unique steps (overruling template at runtime) per file.
  • Internally, there is some complex de-duplication logic to determine how many unique assemblies we need.
  • In another class, AssemblyWatcher, there is more complex logic to track multiple assemblies and match them with their corresponding files.

I can't find any other test or example besides dynamic steps on why we need this complexity instead of just one assembly for all files.

@tim-kos mentioned this:

fields is per Assembly, not per file, as well as steps. We either need to go all in on this and make all of them (template_id, fields and steps) per file, or all of them stay per Assembly. Was there expressed desire of feature requests to make everything very dynamic per file? I am not sure I follow this … makes things very complicated as it’s also against what we are advertising in the API docs. Multiple flows based on file type can be achieved inside the same Template using the /file/filter robot, Assembly variables and the fact that robots ignore file types they cannot handle.

Solution

  • Remove all complexity regarding managing, tracking, and de-duplicating multiple assemblies. Always create one assembly for all files. If I understand correctly, we still upload each file's meta so if anyone wants to do something in their template specifically per file, they can use {file.meta.width} or whatever is custom per file in meta.
  • assemblyOptions should be called once with global Uppy meta.

Alternatives

Find an edge case I'm missing to justify creating assemblies per file.

Murderlon avatar Oct 25 '23 13:10 Murderlon