Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Livewire destroys existing files on a file upload property. #1230

Closed
mattcookdev opened this issue Jul 7, 2020 · 10 comments
Closed

Livewire destroys existing files on a file upload property. #1230

mattcookdev opened this issue Jul 7, 2020 · 10 comments

Comments

@mattcookdev
Copy link

Description

Not really sure if this is considered a bug, an oversight, or deliberately not included so I'll leave it as a bug report for now. Basically, I'm attempting to implement existing file-uploads. I figured the simplest way to do this would be;

  • Any files that already exist on the server are instantiated as an instance of Illuminate\Http\UploadedFile (or my own custom class that extends Illuminate\Http\UploadedFile like Livewire\TemporaryUploadedFile does), and any uploads that are new are instantiated as an instance of Livewire\TemporaryUploadedFile as is default with Livewire. This would allow for relatively easy filtering as database wise we could just check;
  1. Loop over existing records in database and diff against all elements in your livewire property that aren't an instance of Livewire\TemporaryUploadedFile. If they don't exist delete them otherwise keep them.
  2. Collect instances of Livewire\TemporaryUploadedFile and loop over them and store each of them to the database.

However, it would appear if you attempt to use an instance of Illuminate\Http\UploadedFile as part of the array you use for wire:model, it just obliterates your data when you upload a new Livewire\TemporaryUploadedFile that gets added to the array. See example below;

  • Before another upload:
    image

  • After another upload:
    image

As you can see, the Livewire\TemporaryUploadedFile now exists but it completely purged my previous data.

But if I use another type like a string, or even TemporaryUploadedFile::createFromLivewire($i->getPath());, it persists and doesn't get corrupted by the array as shown below.

  • Before another upload:
    image

  • After another upload:
    image

As you can see, it stays in the array but this doesn't seem like a very good way of doing it since TemporaryUploadedFile causes a lot of issues with the path (since I'd assume it wasn't really built for this purpose) as you can see in the first screenshot, and even just from a naming perspective it seems pretty odd to name an already uploaded/saved file as Temporary.

I'd be interested to see if someone has figured out how to solve this on the current version of Livewire or if it will require some internal changes before a use-case like this is viable with Livewire.

Context

  • Livewire version: [1.3.0]
  • Laravel version: [7.18.0]
  • Browser: [Chrome]
@mattcookdev mattcookdev changed the title Livewire destroys existing files on a file upload property Livewire destroys existing files on a file upload property. Jul 7, 2020
@mattcookdev
Copy link
Author

@calebporzio Sorry to ping you, but is there any official/recommended way to do this as of yet, or is this something we can expect in v2?

@thoresuenert
Copy link

The Problem is a strange behaviour in the WithFileUploads Trait in the following function:

    public function finishUpload($name, $tmpPath, $isMultiple)
    {
        $this->cleanupOldUploads();

        if ($isMultiple) {
            $file = collect($tmpPath)->map(function ($i) {
                return TemporaryUploadedFile::createFromLivewire($i);
            })->toArray();
            $this->emitSelf('upload:finished', $name, collect($file)->map->getFilename()->toArray());
        } else {
            $file = TemporaryUploadedFile::createFromLivewire($tmpPath[0]);
            $this->emitSelf('upload:finished', $name, [$file->getFilename()]);

            // If the property is an array, but the upload ISNT set to "multiple"
            // then APPEND the upload to the array, rather than replacing it.
            if (is_array($value = $this->getPropertyValue($name))) {
                $file = array_merge($value, [$file]);
            }
        }

        $this->syncInput($name, $file);
    }

when you have a property in your component defined as array and the upload button isn't set to multiple, the already uploaded files will persist.

// if ! $isMultiple
 if (is_array($value = $this->getPropertyValue($name))) {
                $file = array_merge($value, [$file]);
            }

we overwrite this function. I will post our complete component to show you our complete implementation:
Not exactly the same use case, because we store everything in the session and persists everything later (multi step form) into the database.

class UploadFiles extends Component
{
    use WithFileUploads;

    public $photos;
    public $collectionName;

    public function mount($collectionName = 'images')
    {
        $this->collectionName = $collectionName;
        $this->photos = $this->collection;
    }


    public function updatedPhotos()
    {
        $this->validate([
            'photos.*' => 'image',
        ]);

        $this->saveFilesToSession($this->photos);
    }

    public function finishUpload($name, $tmpPath, $isMultiple)
    {
        $this->cleanupOldUploads();

        $files = collect($tmpPath)->map(function ($i) {
            return TemporaryUploadedFile::createFromLivewire($i);
        })->toArray();
        $this->emitSelf('upload:finished', $name, collect($files)->map->getFilename()->toArray());

        $files = array_merge($this->getPropertyValue($name), $files);
        $this->syncInput($name, $files);
    }

    public function getCollectionProperty()
    {
        return TemporaryUploadedFile::unserializeFromLivewireRequest(session()->get("report.{$this->collectionName}", 'livewire-files:'));
    }

    public function saveFilesToSession($collection)
    {
        session()->put("report.{$this->collectionName}", TemporaryUploadedFile::serializeMultipleForLivewireResponse($collection));
        return $collection;
    }
}

@mattcookdev
Copy link
Author

@thoresuenert Thanks for the reply. Apologies for such a late reply, I've been super busy and honestly forgot about this until now.

I think I follow what you are doing in your code, but applying it to my use-case is pretty tricky and I feel like overriding functions and doing things like hacky casts sorta takes away from the magic of the feature a bit since one of it's main appeals is ease-of-use!

Hopefully Caleb might be able to address this issue directly when he gets some free time since even after months of trying to solve this myself, everything has come up empty and I'm sort of left not really sure how to tackle this problem other than just writing my own entire custom implementation (which I really don't want to do if I can avoid it 😅 ).

@squishythejellyfish
Copy link
Collaborator

👋 Oh Hi! I'm Squishy, the friendly jellyfish that manages Livewire issues.

I see this issue has been closed.

Here in the Livewire repo, we have an "issues can be closed guilt-free and without explanation" policy.

If for ANY reason you think this issue hasn't been resolved, PLEASE feel empowered to re-open it.

Re-opening actually helps us track which issues are a priority.

Reply "REOPEN" to this comment and we'll happily re-open it for you!

(More info on this philosophy here: https://twitter.com/calebporzio/status/1321864801295978497)

@alexfraundorf-com
Copy link

The Problem is a strange behaviour in the WithFileUploads Trait in the following function:

    public function finishUpload($name, $tmpPath, $isMultiple)
    {
        $this->cleanupOldUploads();

        if ($isMultiple) {
            $file = collect($tmpPath)->map(function ($i) {
                return TemporaryUploadedFile::createFromLivewire($i);
            })->toArray();
            $this->emitSelf('upload:finished', $name, collect($file)->map->getFilename()->toArray());
        } else {
            $file = TemporaryUploadedFile::createFromLivewire($tmpPath[0]);
            $this->emitSelf('upload:finished', $name, [$file->getFilename()]);

            // If the property is an array, but the upload ISNT set to "multiple"
            // then APPEND the upload to the array, rather than replacing it.
            if (is_array($value = $this->getPropertyValue($name))) {
                $file = array_merge($value, [$file]);
            }
        }

        $this->syncInput($name, $file);
    }

when you have a property in your component defined as array and the upload button isn't set to multiple, the already uploaded files will persist.

// if ! $isMultiple
 if (is_array($value = $this->getPropertyValue($name))) {
                $file = array_merge($value, [$file]);
            }

we overwrite this function. I will post our complete component to show you our complete implementation:
Not exactly the same use case, because we store everything in the session and persists everything later (multi step form) into the database.

class UploadFiles extends Component
{
    use WithFileUploads;

    public $photos;
    public $collectionName;

    public function mount($collectionName = 'images')
    {
        $this->collectionName = $collectionName;
        $this->photos = $this->collection;
    }


    public function updatedPhotos()
    {
        $this->validate([
            'photos.*' => 'image',
        ]);

        $this->saveFilesToSession($this->photos);
    }

    public function finishUpload($name, $tmpPath, $isMultiple)
    {
        $this->cleanupOldUploads();

        $files = collect($tmpPath)->map(function ($i) {
            return TemporaryUploadedFile::createFromLivewire($i);
        })->toArray();
        $this->emitSelf('upload:finished', $name, collect($files)->map->getFilename()->toArray());

        $files = array_merge($this->getPropertyValue($name), $files);
        $this->syncInput($name, $files);
    }

    public function getCollectionProperty()
    {
        return TemporaryUploadedFile::unserializeFromLivewireRequest(session()->get("report.{$this->collectionName}", 'livewire-files:'));
    }

    public function saveFilesToSession($collection)
    {
        session()->put("report.{$this->collectionName}", TemporaryUploadedFile::serializeMultipleForLivewireResponse($collection));
        return $collection;
    }
}

Thank you for posting this solution. It worked perfectly for my situation of images being deleted when I wanted to add more to the multiple upload.

@gtu-myowin
Copy link

gtu-myowin commented Sep 20, 2021

It works!
Thanks @thoresuenert for digging deeper and sharing your code. Although, he explained well, some people might get overwhelmed by the full component code. Well, don't! The code that makes it possible is this one line of code

$files = array_merge($this->getPropertyValue($name), $files);

but you need to override the default finishUpload function like following to get it work

public function finishUpload($name, $tmpPath)
{
	$this->cleanupOldUploads();

	$files = collect($tmpPath)->map(function ($i) {
		return TemporaryUploadedFile::createFromLivewire($i);
	})->toArray();
	$this->emitSelf('upload:finished', $name, collect($files)->map->getFilename()->toArray());

	// merge it to persist uploaded images
	$files = array_merge($this->getPropertyValue($name), $files);
	$this->syncInput($name, $files);
}

I wonder why Caleb didn't do this in the first place but since he's not giving any warning while closing this thread, I consider it safe to do so.

@sertxudev
Copy link

Note:
If the upload isn't set as "multiple" but the property is an array it will be appended.

// If the property is an array, but the upload ISNT set to "multiple"
// then APPEND the upload to the array, rather than replacing it.
if (is_array($value = $this->getPropertyValue($name))) {
$file = array_merge($value, [$file]);
}


So, if you're uploading the files using the JavaScript API like me you can do the following:

class Form extends Component {

  use WithFileUploads;

  public $attachments = [];
  ...
for (let file of files) {
  @this.upload('attachments', file,
    (files) => console.log('uploaded', files),
    (error) => console.log('error', error),
    (event) => console.log('progress', event.detail.progress)
  )
}

@reasecret
Copy link

@thoresuenert 's reply helped me too. But I'm wondering is there a way to prevent uploading same image?

@lemonotype
Copy link

Is this workaround still supposed to work in 2024 with Livewire v3? I still have the same exact symptoms described here (array entries getting replaced if input set to multiple).

My problem is that finishUpload() doesn't seem to be firing as of v3 and if I just set the function name to updatedPhotos() to make it fire it's not working anyway. I adjusted emitSelf to dispatch but I'm not clever enough to fix syncInput or this functionality in general.

@joshhanley
Copy link
Member

joshhanley commented Feb 8, 2024

@lemonotype in V3 the method has been renamed to _finishUpload(); so if you rename your override, it should work.

function _finishUpload($name, $tmpPath, $isMultiple)
{
$this->cleanupOldUploads();
if ($isMultiple) {
$file = collect($tmpPath)->map(function ($i) {
return TemporaryUploadedFile::createFromLivewire($i);
})->toArray();
$this->dispatch('upload:finished', name: $name, tmpFilenames: collect($file)->map->getFilename()->toArray())->self();
} else {
$file = TemporaryUploadedFile::createFromLivewire($tmpPath[0]);
$this->dispatch('upload:finished', name: $name, tmpFilenames: [$file->getFilename()])->self();
// If the property is an array, but the upload ISNT set to "multiple"
// then APPEND the upload to the array, rather than replacing it.
if (is_array($value = $this->getPropertyValue($name))) {
$file = array_merge($value, [$file]);
}
}
app('livewire')->updateProperty($this, $name, $file);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants