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

FileUploader is triggering 2 uploads to s3 when using ProcessFile prop #5817

Closed
4 tasks done
brandonwatson opened this issue Sep 20, 2024 · 8 comments · Fixed by #5818
Closed
4 tasks done

FileUploader is triggering 2 uploads to s3 when using ProcessFile prop #5817

brandonwatson opened this issue Sep 20, 2024 · 8 comments · Fixed by #5818
Assignees
Labels
bug Something isn't working pending-maintainer-response Issue is pending response from an Amplify UI maintainer pending-review An issue or a feature PR is pending review prior to release StorageManager StorageManager related issue of feature request

Comments

@brandonwatson
Copy link

Before creating a new issue, please confirm:

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: #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

@github-actions github-actions bot added pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Sep 20, 2024
@esauerbo esauerbo added the StorageManager StorageManager related issue of feature request label Sep 20, 2024
@esauerbo
Copy link
Contributor

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

@esauerbo esauerbo added bug Something isn't working and removed pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Sep 20, 2024
@brandonwatson
Copy link
Author

brandonwatson commented Sep 20, 2024 via email

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 20, 2024
@esauerbo
Copy link
Contributor

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

@jordanvn jordanvn removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 20, 2024
@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 20, 2024
@jordanvn jordanvn removed the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 20, 2024
@brandonwatson
Copy link
Author

@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.

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 20, 2024
@calebpollman calebpollman self-assigned this Sep 20, 2024
@brandonwatson
Copy link
Author

@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
Copy link
Member

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

@calebpollman calebpollman added pending-review An issue or a feature PR is pending review prior to release and removed pending-maintainer-response Issue is pending response from an Amplify UI maintainer labels Sep 20, 2024
@esauerbo
Copy link
Contributor

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

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify UI maintainer label Sep 21, 2024
@brandonwatson
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pending-maintainer-response Issue is pending response from an Amplify UI maintainer pending-review An issue or a feature PR is pending review prior to release StorageManager StorageManager related issue of feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants