FileUploader is triggering 2 uploads to s3 when using ProcessFile prop

brandonwatson commented 1 month ago

Before creating a new issue, please confirm:

[X] I have searched for duplicate or closed issues and discussions.
[X] I have tried disabling all browser extensions or using a different browser
[X] I have tried deleting the node_modules folder and reinstalling my dependencies
[X] I have read the guide for submitting bug reports.

On which framework/platform are you having an issue?

React

Which UI component?

Storage

How is your app built?

Next.js

What browsers are you seeing the problem on?

Microsoft Edge

Which region are you seeing the problem in?

us-west-1

Please describe your bug.

Usiing FileUploader from '@aws-amplify/ui-react-storage' and utilizing the ProcessFile prop is causing a double upload. I was on: "@aws-amplify/ui-react-storage": "^3.3.1",

and updating to: "@aws-amplify/ui-react-storage": "^3.3.3",

Did not solve the problem.

I had found this old bug: https://github.com/aws-amplify/amplify-ui/issues/5467, and it appeared that it was closed and fixed back in 3.1.6 but I can replicate this problem.

What's the expected behaviour?

One file upload. I have confirmed that the removal of the ProcessFile prop reverts back to the behavior of one file being uploaded.

I am simply trying to change the file name to something that is UUIDv4 based. Nothing crazy. I can confirm that two files are uploaded to S3. I can confirm that the processFile function is being called twice.

Help us reproduce the bug!

{
    "name": "reintern",
    "version": "0.1.0",
    "private": true,
    "engines": {
        "node": "20.x"
    },
    "scripts": {
        "dev": "next dev",
        "build": "next build",
        "start": "next start",
        "lint": "next lint",
        "populate-sandbox-db": "ts-node --project tsconfig.node.json src/sandbox-utils/populate-sandbox-database.ts"
    },
    "dependencies": {
        "@aws-amplify/adapter-nextjs": "^1.2.10",
        "@aws-amplify/auth": "^6.3.13",
        "@aws-amplify/storage": "^6.6.5",
        "@aws-amplify/ui-react": "^6.1.11",
        "@aws-amplify/ui-react-storage": "^3.3.3",
        "@headlessui/react": "^2.1.2",
        "@heroicons/react": "^2.1.5",
        "aws-amplify": "^6.4.0",
        "aws-sdk": "^2.1659.0",
        "dotenv": "^16.4.5",
        "next": "14.0.4",
        "openai": "^4.62.1",
        "react": "^18",
        "react-dom": "^18",
        "uuid": "^10.0.0"
    },
    "devDependencies": {
        "@aws-amplify/backend": "^1.0.4",
        "@aws-amplify/backend-cli": "^1.1.0",
        "@aws-sdk/client-cognito-identity-provider": "^3.614.0",
        "@types/aws-lambda": "^8.10.141",
        "@types/node": "^20",
        "@types/react": "^18",
        "@types/react-dom": "^18",
        "autoprefixer": "^10.4.19",
        "aws-cdk": "^2.148.0",
        "aws-cdk-lib": "^2.148.0",
        "constructs": "^10.3.0",
        "daisyui": "^4.12.10",
        "esbuild": "^0.23.0",
        "eslint": "^8",
        "eslint-config-next": "14.0.4",
        "postcss": "^8.4.39",
        "tailwindcss": "^3.4.4",
        "ts-node": "^10.9.2",
        "tsx": "^4.16.2",
        "typescript": "^5.5.3"
    }
}

'use client';

import '@aws-amplify/ui-react/styles.css';
import { useState, useRef } from 'react';
import { v4 as uuidv4 } from 'uuid';
import { FileUploader } from '@aws-amplify/ui-react-storage';
import {
    createImageRecord,
    deleteUploadedFiles,
} from '../app/propertyedit/propertyEditServerActions';

interface ImageUploadModalProps {
    isOpen: boolean;
    onClose: () => void;
    onUploadComplete: (newImages: { imageId: string; key: string }[]) => void;
    propertyId: string;
    roomId: string;
    identityId: string;
}

interface UploadedFile {
    imageId: string;
    key: string;
}

export default function ImageUploadModal({
    isOpen,
    onClose,
    onUploadComplete,
    propertyId,
    roomId,
    identityId,
}: ImageUploadModalProps) {
    const [uploadedFiles, setUploadedFiles] = useState<UploadedFile[]>([]);
    const [uploadingFiles, setUploadingFiles] = useState<Set<string>>(
        new Set()
    );

    const maxFileSize = 15000000; // 15MB limit

    const processedFiles = useRef(new Set<string>());

    async function processFile({
        file,
    }: {
        file: File;
    }): Promise<{ file: File; key: string }> {
        console.log(`LOGGING: inside processFile - file: ${file.name}`);
        const fileExtension = file.name.split('.').pop();
        const newKey = `${uuidv4()}.${fileExtension}`;

        // Create a new File object with the new name
        const newFile = new File([file], newKey, { type: file.type });

        return { file: newFile, key: newKey };
    }

    function handleUploadStart({ key }: { key?: string }) {
        console.log(`LOGGING: inside handleUploadStart - key: ${key}`);
        if (key) {
            setUploadingFiles((prev) => new Set(prev).add(key));
        }
    }

    function handleUploadSuccess({ key }: { key?: string }) {
        if (!key) {
            console.error('Upload successful but key is undefined');
            return;
        }

        const imageId = key.split('/').pop() || '';
        console.log(
            `LOGGING: Upload successful key: ${key}, imageId: ${imageId}`
        );

        setUploadedFiles((prev) => [...prev, { imageId, key }]);
        setUploadingFiles((prev) => {
            const newSet = new Set(prev);
            newSet.delete(key);
            return newSet;
        });
    }

    function handleUploadError(error: string, { key }: { key?: string }) {
        console.error(`Error uploading file ${key}:`, error);
        if (key) {
            setUploadingFiles((prev) => {
                const newSet = new Set(prev);
                newSet.delete(key);
                return newSet;
            });
        }
    }

    function handleFileRemove({ key }: { key?: string }) {
        if (key) {
            setUploadedFiles((prev) => prev.filter((file) => file.key !== key));
            setUploadingFiles((prev) => {
                const newSet = new Set(prev);
                newSet.delete(key);
                return newSet;
            });
        }
    }

    async function handleCancel() {
        if (uploadedFiles.length > 0) {
            try {
                await deleteUploadedFiles({
                    keys: uploadedFiles.map((file) => file.key),
                    propertyId,
                    roomId,
                });
            } catch (error) {
                console.error('Error deleting uploaded files:', error);
            }
        }
        setUploadedFiles([]);
        setUploadingFiles(new Set());

        processedFiles.current.clear();
        onClose();
    }

    async function handleOk() {
        for (const file of uploadedFiles) {
            try {
                await createImageRecord(file.imageId, roomId);
            } catch (error) {
                console.error('Error creating image record:', error);
            }
        }
        onUploadComplete(uploadedFiles);
        setUploadedFiles([]);
        setUploadingFiles(new Set());

        processedFiles.current.clear();
        onClose();
    }

    const isUploading = uploadingFiles.size > 0;
    const hasUploadedFiles = uploadedFiles.length > 0;

    if (!isOpen) return null;

    return (
        <div className="fixed inset-0 bg-black bg-opacity-50 flex justify-center items-center z-50">
            <div className="bg-white p-6 rounded-lg w-full max-w-2xl mx-4 max-h-[90vh] overflow-y-auto">
                <h3 className="text-lg font-bold mb-4">Upload Images</h3>
                <FileUploader
                    acceptedFileTypes={['image/*']}
                    maxFileCount={5}
                    maxFileSize={maxFileSize}
                    onUploadStart={handleUploadStart}
                    onUploadSuccess={handleUploadSuccess}
                    onUploadError={handleUploadError}
                    onFileRemove={handleFileRemove}
                    path={({ identityId }) =>
                        `protected/images/${identityId}/${propertyId}/${roomId}/`
                    }
                    processFile={processFile}
                />
                <div className="flex justify-end mt-4">
                    <button
                        onClick={handleCancel}
                        className="mr-2 px-4 py-2 text-gray-600"
                    >
                        Cancel
                    </button>
                    <button
                        onClick={handleOk}
                        className={`px-4 py-2 bg-blue-500 text-white rounded ${
                            !hasUploadedFiles || isUploading
                                ? 'opacity-50 cursor-not-allowed'
                                : ''
                        }`}
                        disabled={!hasUploadedFiles || isUploading}
                    >
                        OK
                    </button>
                </div>
            </div>
        </div>
    );
}

Code Snippet

// Put your code below this line.

Console log output

LOGGING: inside processFile - file: kitchen2.png
ImageUploadModal.tsx:48 LOGGING: inside processFile - file: kitchen2.png
ImageUploadModal.tsx:59 LOGGING: inside handleUploadStart - key: protected/images/{identityId}/d58e4db3-5a08-4204-845f-0f7546b88e67/5c17eea8-1b96-4982-9ec6-0ee2bdc167d5/b503f635-31b9-471d-ae72-06fe4efa8fa8.png
ImageUploadModal.tsx:59 LOGGING: inside handleUploadStart - key: protected/images/{identityId}/d58e4db3-5a08-4204-845f-0f7546b88e67/5c17eea8-1b96-4982-9ec6-0ee2bdc167d5/e8b121cd-c153-42fa-ad7d-443eafa049c2.png
ImageUploadModal.tsx:72 LOGGING: Upload successful key: protected/images/{identityId}/d58e4db3-5a08-4204-845f-0f7546b88e67/5c17eea8-1b96-4982-9ec6-0ee2bdc167d5/e8b121cd-c153-42fa-ad7d-443eafa049c2.png, imageId: e8b121cd-c153-42fa-ad7d-443eafa049c2.png
ImageUploadModal.tsx:72 LOGGING: Upload successful key: protected/images/{identityId}/d58e4db3-5a08-4204-845f-0f7546b88e67/5c17eea8-1b96-4982-9ec6-0ee2bdc167d5/b503f635-31b9-471d-ae72-06fe4efa8fa8.png, imageId: b503f635-31b9-471d-ae72-06fe4ef

Additional information and screenshots

No response

esauerbo commented 1 month ago

Hey @brandonwatson sorry you're experiencing this issue. Thanks for providing all this information. I was able to reproduce this and confirmed that removing the processFile prop fixes the issue. We'll start working on a fix and will provide updates here.

I noticed when I refresh the page it works as expected, it's only after one file has already been uploaded that it becomes an issue. Are you seeing that as well?

FYI @tytremblay @OperationalFallacy

brandonwatson commented 1 month ago

Can you explain that second paragraph a little better? I guess I haven't been cold loading the page after a recompile, restarting NPM run Dev, I don't think. But I don't know the refreshing at anything to do with it. But I can certainly check that if you can be a bit more explicit with your repro steps.

From: Emma Sauerborn @.> Sent: Friday, September 20, 2024 9:12:34 AM To: aws-amplify/amplify-ui @.> Cc: Brandon Watson @.>; Mention @.> Subject: Re: [aws-amplify/amplify-ui] FileUploader is triggering 2 uploads to s3 when using ProcessFile prop (Issue #5817)

Hey @brandonwatsonhttps://github.com/brandonwatson sorry you're experiencing this issue. Thanks for providing all this information. I was able to reproduce this and confirmed that removing the processFile prop fixes the issue. We'll start working on a fix and will provide updates here.

I noticed when I refresh the page it works as expected, it's only after one file has already been uploaded that it becomes an issue. Are you seeing that as well?

FYI @tytremblayhttps://github.com/tytremblay @OperationalFallacyhttps://github.com/OperationalFallacy

— Reply to this email directly, view it on GitHubhttps://github.com/aws-amplify/amplify-ui/issues/5817#issuecomment-2363960853, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AABV5JJDHLHLHWMHQ4INRFTZXQ3ODAVCNFSM6AAAAABORAUZO2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRTHE3DAOBVGM. You are receiving this because you were mentioned.Message ID: @.***>

esauerbo commented 1 month ago

@brandonwatson Yeah I just meant hitting the refresh button on the web browser and then uploading a file.

brandonwatson commented 1 month ago

@tytremblay thanks for the heads up. When I was director of developer platform for Windows Phone I was a pretty public figure and had no problem publishing my phone number. Who's one of my core team principles about being universal accessible to our customers. I'm sure my Colorado number is out there now with everything I do, but I went ahead and removed it all the same.

brandonwatson commented 1 month ago

@brandonwatson Yeah I just meant hitting the refresh button on the web browser and then uploading a file.

I tried your repro and hit refresh. This did not change anything in terms of outcome. I still had two file uploads.

Some more project info, in case this is helpful - and please ask for any additional data.

First and foremost, I am a self-taught dev, and this project was being used to teach me TypeScript, so just keep that in mind.
I am a former L8 Product Manager with AMZN
I am using SSR wherever possible and pushing "use client" as low in my render tree as possible
In this instance, I have a top level page that is a protected route.
- The only place in my app where I wrap anything with Authenticator is in /auth/page.tsx, which is where I redirect my protected routes from my middleware.
This page.tsx renders inside of it a client component (PropertyEditForm.tsx) which is a wrapper for the interactive content on the page. page.tsx passes returned server called database content to PropertyEditForm.
The PropertyEditForm uses the passed in data to fill items on the page, one of which is a map function that calls another client component (PropertyRoom.tsx)
PropertyRoom.tsx contains the component ImageUploadModal, which is the code I supplied in the original report.
ImageUploadModal is what is causing the double file upload.
Inside PropertyRoom I am also making a db call from a useEffect to get data which was not passed down from the original query. This is not a long term solution, and I need to update my selectionSet from the original DB call to make sure that the useEffect gathered data arrives with the props from page.tsx. I was still very much coming to grips with where server and client boundaries were, and how to go about getting data, subscribing to data, etc, and this was where I ended up. Looking at it now, I know I need to fix what data is coming down in the props and not use useEffect to gather additional data.

UPDATE

I refactored my code and moved the useEffect up to PropertyEditRoom and removed it from PropertyRoom. The removal of the useEffect from PropertyRoom had no impact on the FileUploader issue. It is still uploading two files.

calebpollman commented 1 month ago

@brandonwatson Was able to repro the issue, we are working on a fix now

esauerbo commented 1 month ago

This has been fixed in the latest version @aws-amplify/ui-react-storage@3.3.4 https://www.npmjs.com/package/@aws-amplify/ui-react-storage/v/3.3.4

brandonwatson commented 1 month ago

I have removed 3.3.3, and ran npm install @aws-amplify/ui-react-storage@latest which updated to 3.3.4.

For anyone else who had this issue and was updating, this is not sufficient.

I had to take the extra step of perm deleting node_module directory and running npm install for this change to take effect.

3.3.4 appears to no longer double upload files.

aws-amplify / amplify-ui