@uppy/core: allow duplicate files with onBeforeFileAdded

[Murderlon]

Closes #4470

• Change default signature of onBeforeFileAdded to check for duplicate files to not make this a breaking change.
• Swap order of checkIfFileAlreadyExists and onBeforeFileAdded.

This means:

• If you add the same file again before uploading, it will override it.
• If you add the same file after uploading, you can upload it again. Depending on your uploader and backend this may or may not override the file. Tus for instance always generates unique IDs so it wouldn't override it. But people should be able to handle it the way they want.

Needs docs PR too.

[aduh95]

Do we have a test for the default behavior of adding the same file twice? If not, can we add that test case?

[Murderlon]

Do we have a test for the default behavior of adding the same file twice? If not, can we add that test case?

uppy/packages/@uppy/core/src/Uppy.test.js

Lines 775 to 840 in 7847708

	it('should not allow a dupicate file, a file with the same id', () => {
	const core = new Core()
	const sameFileBlob = new File([sampleImage], { type: 'image/jpeg' })
	core.addFile({
	source: 'jest',
	name: 'foo.jpg',
	type: 'image/jpeg',
	data: sameFileBlob,
	})
	expect(() => {
	core.addFile({
	source: 'jest',
	name: 'foo.jpg',
	type: 'image/jpeg',
	data: sameFileBlob,
	meta: {
	notARelativePath: 'folder/a',
	},
	})
	}).toThrow(
	"Cannot add the duplicate file 'foo.jpg', it already exists",
	)
	expect(core.getFiles().length).toEqual(1)
	})
	it('should allow a duplicate file if its relativePath is different, thus the id is different', () => {
	const core = new Core()
	core.addFile({
	source: 'jest',
	name: 'foo.jpg',
	type: 'image/jpeg',
	data: new File([sampleImage], { type: 'image/jpeg' }),
	})
	core.addFile({
	source: 'jest',
	name: 'foo.jpg',
	type: 'image/jpeg',
	data: new File([sampleImage], { type: 'image/jpeg' }),
	meta: {
	relativePath: 'folder/a',
	},
	})
	expect(core.getFiles().length).toEqual(2)
	})
	it('should not allow a file if onBeforeFileAdded returned false', () => {
	const core = new Core({
	onBeforeFileAdded: (file) => {
	if (file.source === 'jest') {
	return false
	}
	return undefined
	},
	})
	expect(() => {
	core.addFile({
	source: 'jest',
	name: 'foo.jpg',
	type: 'image/jpeg',
	data: new File([sampleImage], { type: 'image/jpeg' }),
	})
	}).toThrow(
	'Cannot add the file because onBeforeFileAdded returned false.',
	)
	expect(core.getFiles().length).toEqual(0)
	})

[aduh95]

Would it make sense to put those two tests next to each other?

[Murderlon]

It's already in the same describe block about adding new files, just below the other test for onBeforeFileAdded.

[aduh95]

I think it would make more sense to have the two tests next to one another, it would also be the occasion to fix the typo dupicate -> duplicate, but no strong feelings

[mifi]

othar than that, lgtm!