Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions src/file-selector.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {FileWithPath} from './file';
import { FileWithPath } from './file';
import {fromEvent} from './file-selector';

const toFileWithPathSpy = jest.spyOn(jest.requireActual('./file'), 'toFileWithPath');

it('returns a Promise', async () => {
const evt = new Event('test');
Expand Down Expand Up @@ -109,6 +109,37 @@ it('should return files from DataTransfer {items} if the passed event is a DragE
expect(file.path).toBe(name);
});

it('should call toFilePath with undefined path if {webkitGetAsEntry} is not a function', async () => {
toFileWithPathSpy.mockClear();
const name = 'test.json';
const mockFile = createFile(name, {ping: true}, {
type: 'application/json'
});

const item = dataTransferItemFromFile(mockFile);
const evt = dragEvtFromFilesAndItems([], [item]);
await fromEvent(evt);
expect(toFileWithPathSpy).toBeCalledTimes(1);
expect(toFileWithPathSpy).toBeCalledWith(mockFile, undefined);
});

it('should call toFilePath with {fullPath} path if file can be converted into an Entry', async () => {
toFileWithPathSpy.mockClear();
const name = 'test.json';
const fullPath = '/testfolder/test.json'
const mockFile = createFile(name, {ping: true}, {
type: 'application/json'
});

const file = fileSystemFileEntryFromFile(mockFile);
file.fullPath = fullPath
const item = dataTransferItemFromEntry(file, mockFile);
const evt = dragEvtFromFilesAndItems([], [item]);
await fromEvent(evt);
expect(toFileWithPathSpy).toBeCalledTimes(1);
expect(toFileWithPathSpy).toBeCalledWith(mockFile, fullPath);
});

it('skips DataTransfer {items} that are of kind "string"', async () => {
const name = 'test.json';
const mockFile = createFile(name, {ping: true}, {
Expand Down Expand Up @@ -445,6 +476,7 @@ interface DirEntry extends Entry {
interface Entry {
isDirectory: boolean;
isFile: boolean;
fullPath?: string;
}

interface DirReader {
Expand Down
8 changes: 7 additions & 1 deletion src/file-selector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,16 @@ function flatten<T>(items: any[]): T[] {

function fromDataTransferItem(item: DataTransferItem) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function fromDataTransferItem(item: DataTransferItem) {
function fromDataTransferItem(item: DataTransferItem, entry?: FileSystemEntry) {

const file = item.getAsFile();

let fileAsEntry: FileSystemEntry | null = null;
if (typeof item.webkitGetAsEntry === 'function') {
fileAsEntry = item.webkitGetAsEntry();
}

Comment on lines +125 to +130
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get rid of this if on line 111 you add this:

else if (entry) {
  return fromDataTransferItem(item, entry);
}

Because you already have the entry in the toFilePromises func.

if (!file) {
return Promise.reject(`${item} is not a File`);
}
const fwp = toFileWithPath(file);
const fwp = toFileWithPath(file, fileAsEntry?.fullPath ?? undefined);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const fwp = toFileWithPath(file, fileAsEntry?.fullPath ?? undefined);
const fwp = toFileWithPath(file, entry?.fullPath ?? undefined);

return Promise.resolve(fwp);
}

Expand Down
45 changes: 45 additions & 0 deletions src/file.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,51 @@ describe('toFile()', () => {
expect(fileWithPath.path).toBe(path);
});

it('sets the {relativePath} if provided without overwriting {path}', () => {
const fullPath = '/Users/test/Desktop/test/test.json';
const path = '/test/test.json';
const file = new File([], 'test.json');

// @ts-expect-error
file.path = fullPath;
const fileWithPath = toFileWithPath(file, path);
expect(fileWithPath.path).toBe(fullPath);
expect(fileWithPath.relativePath).toBe(path);
});

test('{relativePath} is enumerable', () => {
const path = '/test/test.json';
const file = new File([], 'test.json');
const fileWithPath = toFileWithPath(file, path);

expect(Object.keys(fileWithPath)).toContain('relativePath');

const keys = [];
for (const key in fileWithPath) {
keys.push(key);
}

expect(keys).toContain('relativePath');
});

it('uses the File {webkitRelativePath} as {relativePath} if it exists', () => {
const name = 'test.json';
const path = 'test/test.json';
const file = new File([], name);
Object.defineProperty(file, 'webkitRelativePath', {
value: path
});
const fileWithPath = toFileWithPath(file);
expect(fileWithPath.relativePath).toBe(path);
});

it('uses the File {name} as {relativePath} if not provided and prefix with forward slash (/)', () => {
const name = 'test.json';
const file = new File([], name);
const fileWithPath = toFileWithPath(file);
expect(fileWithPath.relativePath).toBe('/' + name);
});

it('sets the {type} from extension', () => {
const types = Array.from(COMMON_MIME_TYPES.values());
const files = Array.from(COMMON_MIME_TYPES.keys())
Expand Down
18 changes: 17 additions & 1 deletion src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ export const COMMON_MIME_TYPES = new Map([

export function toFileWithPath(file: FileWithPath, path?: string): FileWithPath {
const f = withMimeType(file);
const {webkitRelativePath} = file;
if (typeof f.path !== 'string') { // on electron, path is already set to the absolute path
const {webkitRelativePath} = file;
Object.defineProperty(f, 'path', {
value: typeof path === 'string'
? path
Expand All @@ -104,11 +104,27 @@ export function toFileWithPath(file: FileWithPath, path?: string): FileWithPath
});
}

// Always populate a relative path so that even electron apps have access to a relativePath value
Object.defineProperty(f, 'relativePath', {
value: typeof path === 'string'
? path
// If <input webkitdirectory> is set,
// the File will have a {webkitRelativePath} property
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/webkitdirectory
: typeof webkitRelativePath === 'string' && webkitRelativePath.length > 0
? webkitRelativePath
: `/${file.name}`, // prepend forward slash (/) to ensure consistancy when path isn't supplied.
writable: false,
configurable: false,
enumerable: true
})
Comment on lines +108 to +120
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not too different from setting the path. Let's make this block a function and reuse it for setting the path and relativePath.

Also, since the path is relative, /${file.name} should probably be ./${file.name}.


return f;
}

export interface FileWithPath extends File {
readonly path?: string;
readonly relativePath?: string;
}

function withMimeType(file: FileWithPath) {
Expand Down