Problem: Last onFileAdd wins

[gossi]

Hey,

in my current screen I'm developing, I have two UI locations where I am triggering an upload and a third location, that is showing the upload (and has an upload as well). In short, the code is looking like this:

<div>
  <FileUpload @name="my-queue" @onClick={{this.firstUploadHandler}}>Upload in location 1</FileUpload>
</div>

...

<div>
  <FileUpload @name="my-queue" @onClick={{this.secondUploadHandler}}>Upload in location 2</FileUpload>
</div>

...

<div>
  {{#let (file-queue name="my-queue") as |queue|}}
    {{#each queue.files as |file|}}
      ... ui for {{file}}
    {{/each}}
    <FileUpload @name={{queue.name}} @onClick={{this.thirdUploadHandler}}>Upload in location 3</FileUpload>
  {{/let}}
</div>

You may realize, there are firstUploadHandler, secondUploadHandler and thirdUploadHandler. That is, I want to give feedback about the upload in the respective locations. Let's say, user decided to use location 1 for upload, I wanna show progress there. If user decided to use location 2 for upload, I want to show progress over there and so forth.

And this is where the problem comes in. All locations operate on the same queue, which is intended. Each <FileUpload> component will overwrite onFileAdd in the queue with the value from @onClick, means the last @onClick wins. Which is, if location 1 is used for uploading, the thirdUploadHandler is triggered.

That comes with a problem, meaning the @onClick component is not bound to the component, but the queue and the @onClick cannot be trusted to work "as intended". I've seen in #316 that this will likely improve it, by properly connecting it to the queue through the helper, I think that's the much better API. I can only guess, that the queue is taking over the responsibility of transmitting the file, while a component - such as <FileSelect> with the @onSelected argument is there for handling UI responsibility? Am I reading this correctly and is this expected to come?

What would be my best options to handle this with v4 right now?

[gilest]

Hey – thanks for reporting

Yes 😅 I'm afraid you're right, although I'm not sure about the @onClick argument in your examples. Presumably you mean @onFileAdd.

I (maintainer, but not original author) just recently noticed this when migrating to Octane in #514

Queue instances and Components

When FileUpload/FileDropzone call didRecieveAttrs they directly set some properties on their queue.

I've refactored this to an update-queue modifier.

  {{update-queue
    this.queue
    accept=@accept
    multiple=@multiple
    disabled=@disabled
    onFileAdd=this.onFileAdd
  }}

When testing this I noticed that it's an unsafe operation – the queue only has a single property for each of these and there's no guarantee about the order in which properties might overwrite each other. (For example when rendering a dropzone and a file select for the same queue).

I'm comfortable releasing as-is for now (believe it's the same as existing functionality) – but we should reconsider this API in the future.

Since you asked.

What would be my best options to handle this with v4 right now?

If we can get a fix staged for this we can release it for you as a5.0.0-beta.x this would be helpful because need users to test the new betas before we release the next major.

If you're interested I would welcome a PR to solve this 🙇🏻 if not I'll try and get to it soon(™️)

Steps I would take:

• Add failing test which uses multiple @onFileAdd arguments and expects them all to be called by an upload
• Adjust addon/queue.js so that it can store multiple onFileAdd callback functions and call them all in _addFiles()
• Adjust addon/modifiers/update-queue.js to
  • Add its callback to the queue
  • Remove its callback from the queue when the modified element is removed from the DOM (see cleanup examples in ember-modifier)
• Ensure all tests pass ✅

[gossi]

Thanks @gilest for your answer

Yes 😅 I'm afraid you're right, although I'm not sure about the @onClick argument in your examples. Presumably you mean @onFileAdd.

Yes, you are right, I mean @onFileAdd 🙈

About your suggested solution:

Adjust addon/queue.js so that it can store multiple onFileAdd callback functions and call them all in _addFiles()

Whilst technically feasible, this wouldn't make the components API work. If there are three components and an upload is triggered in one location, then all callbacks would be invoked. That may be ok by design (because at least the owerwrite wouldn't happen any longer), yet wouldn't fit my particular use-case ... and both are somewhat valid :D It's a tricky piece of an API, I must admit - better not make a shot from the hip and give it some thought instead.

I see there are three levels at which you want to interact:

1. Superglobal: All queues/files (unnamed queue)
2. Global: Named queue and its filese
3. Local: UI Element being part of one (or more?) queues

I think, it is ok to ignore superglobal here and think about the interaction of global and local. It should be explicit if I add callbacks to queue or ui element. Depending on where I add that one, it will react accordingly. Adding a callback to the queue, will always notify me whenever anything is happening in that queue. Addding a callback to the UI element will only notify me when anything from the UI element is happening. The UI element will still add elements to its associated queue, but will only get notified back, if this is from the same origin.

That's my understanding? That's how I'd interact with those mechanics, as this is how the rest of the ember ecosystem works. Please verify or make comments onto that. If this is valid, then we can think about an API design =)

Interesting topic that came up =) Also hit me up on discord if you prefer.

[gilest]

That may be ok by design (because at least the overwrite wouldn't happen any longer), yet wouldn't fit my particular use-case.

Right – I didn't completely understand your use-case (and was a little mislead by the title of the issue). My suggestion was to fix a bug in the current API (the callbacks overwriting each other) because I thought it would unblock you before the new API is rolled out.

New API has component and queue callbacks planned.

[gilest]

@gossi FYI I've added an onSelected callback to the FileUpload component as part of (#578). Will release a beta if it is approved and merged.

This was always part of the plan. The awkward @onFileAdd handling is a legacy hangover.

[gilest]

@gossi There's a 5.0.0-beta.0 release which includes the new callbacks from #578. Hope that helps

[gossi]

Amazing, thank you. Will give it a try next week.