feedback 5

This commit is contained in:
Max Schmitt 2024-06-11 23:26:52 +02:00
parent f8728919b3
commit ea9269b426
8 changed files with 66 additions and 36 deletions

View file

@ -250,7 +250,7 @@ export function convertSelectOptionValues(values: string | api.ElementHandle | S
return { options: values as SelectOption[] }; return { options: values as SelectOption[] };
} }
type SetInputFilesFiles = Pick<channels.ElementHandleSetInputFilesParams, 'payloads' | 'localPaths'| 'localDirectory' | 'streams'>; type SetInputFilesFiles = Pick<channels.ElementHandleSetInputFilesParams, 'payloads' | 'localPaths' | 'localDirectory' | 'streams' | 'directoryStream'>;
function filePayloadExceedsSizeLimit(payloads: FilePayload[]) { function filePayloadExceedsSizeLimit(payloads: FilePayload[]) {
return payloads.reduce((size, item) => size + (item.buffer ? item.buffer.byteLength : 0), 0) >= fileUploadSizeLimit; return payloads.reduce((size, item) => size + (item.buffer ? item.buffer.byteLength : 0), 0) >= fileUploadSizeLimit;
@ -262,36 +262,46 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
if (items.some(item => typeof item === 'string')) { if (items.some(item => typeof item === 'string')) {
if (!items.every(item => typeof item === 'string')) if (!items.every(item => typeof item === 'string'))
throw new Error('File paths cannot be mixed with buffers'); throw new Error('File paths cannot be mixed with buffers');
const itemFileTypes = (await Promise.all((items as string[]).map(async item => (await fs.promises.stat(item)).isDirectory() ? 'directory' : 'file'))); let localPaths: string[] | undefined;
if (new Set(itemFileTypes).size > 1 || itemFileTypes.filter(type => type === 'directory').length > 1) let localDirectory: string | undefined;
for (const item of items) {
const stat = await fs.promises.stat(item as string);
if (stat.isDirectory()) {
if (localDirectory)
throw new Error('Multiple directories are not supported');
localDirectory = path.resolve(item as string);
} else {
localPaths ??= [];
localPaths.push(path.resolve(item as string));
}
}
if (localPaths?.length && localDirectory)
throw new Error('File paths must be all files or a single directory'); throw new Error('File paths must be all files or a single directory');
if (context._connection.isRemote()) { if (context._connection.isRemote()) {
const streams = (await Promise.all((items).map(async item => { const files = localDirectory ? (await fs.promises.readdir(localDirectory, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(f.path, f.name)) : localPaths!;
const isDirectory = (await fs.promises.stat(item as string)).isDirectory(); const { writableStreams, rootDir } = await context._wrapApiCall(async () => context._channel.createTempFiles({
const files = isDirectory ? (await fs.promises.readdir(item as string, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(f.path, f.name)) : [item]; rootDirName: localDirectory ? path.basename(localDirectory as string) : undefined,
const { writableStreams, rootDir } = await context._wrapApiCall(async () => context._channel.createTempFiles({ items: await Promise.all(files.map(async file => {
rootDirName: isDirectory ? path.basename(item as string) : undefined, const lastModifiedMs = (await fs.promises.stat(file)).mtimeMs;
items: await Promise.all((files as string[]).map(async file => { return {
const lastModifiedMs = (await fs.promises.stat(item as string)).mtimeMs; name: localDirectory ? path.relative(localDirectory as string, file) : path.basename(file),
return { lastModifiedMs
name: isDirectory ? path.relative(item as string, file) : path.basename(file), };
lastModifiedMs })),
}; }), true);
})), for (let i = 0; i < files.length; i++) {
}), true); const writable = WritableStream.from(writableStreams[i]);
for (let i = 0; i < files.length; i++) { await pipelineAsync(fs.createReadStream(files[i]), writable.stream());
const writable = WritableStream.from(writableStreams[i]); }
await pipelineAsync(fs.createReadStream((files as string[])[i]), writable.stream()); return {
} directoryStream: rootDir,
return rootDir ?? writableStreams; streams: localDirectory ? undefined : writableStreams,
}))).flat(); };
return { streams };
} }
const resolvedItems = (items as string[]).map(f => path.resolve(f));
return { return {
localPaths: resolvedItems.filter((_, i) => itemFileTypes[i] === 'file'), localPaths,
localDirectory: resolvedItems.find((_, i) => itemFileTypes[i] === 'directory'), localDirectory,
}; };
} }

View file

@ -1631,6 +1631,7 @@ scheme.FrameSetInputFilesParams = tObject({
buffer: tBinary, buffer: tBinary,
}))), }))),
localDirectory: tOptional(tString), localDirectory: tOptional(tString),
directoryStream: tOptional(tChannel(['WritableStream'])),
localPaths: tOptional(tArray(tString)), localPaths: tOptional(tArray(tString)),
streams: tOptional(tArray(tChannel(['WritableStream']))), streams: tOptional(tArray(tChannel(['WritableStream']))),
timeout: tOptional(tNumber), timeout: tOptional(tNumber),
@ -1999,6 +2000,7 @@ scheme.ElementHandleSetInputFilesParams = tObject({
buffer: tBinary, buffer: tBinary,
}))), }))),
localDirectory: tOptional(tString), localDirectory: tOptional(tString),
directoryStream: tOptional(tChannel(['WritableStream'])),
localPaths: tOptional(tArray(tString)), localPaths: tOptional(tArray(tString)),
streams: tOptional(tArray(tChannel(['WritableStream']))), streams: tOptional(tArray(tChannel(['WritableStream']))),
timeout: tOptional(tNumber), timeout: tOptional(tNumber),

View file

@ -57,8 +57,4 @@ export class WritableStreamDispatcher extends Dispatcher<{ guid: string, streamO
return this._object.streamOrDirectory; return this._object.streamOrDirectory;
return this._object.streamOrDirectory.path as string; return this._object.streamOrDirectory.path as string;
} }
isDirectory(): boolean {
return typeof this._object.streamOrDirectory === 'string';
}
} }

View file

@ -652,6 +652,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
await Promise.all((localPathsOrDirectory).map(localPath => ( await Promise.all((localPathsOrDirectory).map(localPath => (
fs.promises.access(localPath, fs.constants.F_OK) fs.promises.access(localPath, fs.constants.F_OK)
))); )));
// Browsers traverse the given directory asynchronously and we want to ensure all files are uploaded.
const waitForInputEvent = localDirectory ? this.evaluate(node => new Promise<any>(fulfill => { const waitForInputEvent = localDirectory ? this.evaluate(node => new Promise<any>(fulfill => {
node.addEventListener('input', fulfill, { once: true }); node.addEventListener('input', fulfill, { once: true });
})).catch(() => {}) : Promise.resolve(); })).catch(() => {}) : Promise.resolve();

View file

@ -30,16 +30,17 @@ async function filesExceedUploadLimit(files: string[]) {
} }
export async function prepareFilesForUpload(frame: Frame, params: channels.ElementHandleSetInputFilesParams): Promise<InputFilesItems> { export async function prepareFilesForUpload(frame: Frame, params: channels.ElementHandleSetInputFilesParams): Promise<InputFilesItems> {
const { payloads, streams } = params; const { payloads, streams, directoryStream } = params;
let { localPaths, localDirectory } = params; let { localPaths, localDirectory } = params;
if ([payloads, localPaths, streams].filter(Boolean).length !== 1) if ([payloads, localPaths, localDirectory, streams, directoryStream].filter(Boolean).length !== 1)
throw new Error('Exactly one of payloads, localPaths and streams must be provided'); throw new Error('Exactly one of payloads, localPaths and streams must be provided');
if (streams) { if (streams)
localPaths = streams.map(c => (c as WritableStreamDispatcher)).filter(s => !s.isDirectory()).map(s => s.path()); localPaths = streams.map(c => (c as WritableStreamDispatcher)).map(s => s.path());
localDirectory = streams.map(c => (c as WritableStreamDispatcher)).filter(s => s.isDirectory()).map(s => s.path())[0]; if (directoryStream)
} localDirectory = (directoryStream as WritableStreamDispatcher | undefined)?.path();
if (localPaths) { if (localPaths) {
for (const p of localPaths) for (const p of localPaths)
assert(path.isAbsolute(p) && path.resolve(p) === p, 'Paths provided to localPaths must be absolute and fully resolved.'); assert(path.isAbsolute(p) && path.resolve(p) === p, 'Paths provided to localPaths must be absolute and fully resolved.');

View file

@ -2918,6 +2918,7 @@ export type FrameSetInputFilesParams = {
buffer: Binary, buffer: Binary,
}[], }[],
localDirectory?: string, localDirectory?: string,
directoryStream?: WritableStreamChannel,
localPaths?: string[], localPaths?: string[],
streams?: WritableStreamChannel[], streams?: WritableStreamChannel[],
timeout?: number, timeout?: number,
@ -2931,6 +2932,7 @@ export type FrameSetInputFilesOptions = {
buffer: Binary, buffer: Binary,
}[], }[],
localDirectory?: string, localDirectory?: string,
directoryStream?: WritableStreamChannel,
localPaths?: string[], localPaths?: string[],
streams?: WritableStreamChannel[], streams?: WritableStreamChannel[],
timeout?: number, timeout?: number,
@ -3544,6 +3546,7 @@ export type ElementHandleSetInputFilesParams = {
buffer: Binary, buffer: Binary,
}[], }[],
localDirectory?: string, localDirectory?: string,
directoryStream?: WritableStreamChannel,
localPaths?: string[], localPaths?: string[],
streams?: WritableStreamChannel[], streams?: WritableStreamChannel[],
timeout?: number, timeout?: number,
@ -3556,6 +3559,7 @@ export type ElementHandleSetInputFilesOptions = {
buffer: Binary, buffer: Binary,
}[], }[],
localDirectory?: string, localDirectory?: string,
directoryStream?: WritableStreamChannel,
localPaths?: string[], localPaths?: string[],
streams?: WritableStreamChannel[], streams?: WritableStreamChannel[],
timeout?: number, timeout?: number,

View file

@ -2197,6 +2197,7 @@ Frame:
mimeType: string? mimeType: string?
buffer: binary buffer: binary
localDirectory: string? localDirectory: string?
directoryStream: WritableStream?
localPaths: localPaths:
type: array? type: array?
items: string items: string
@ -2758,6 +2759,7 @@ ElementHandle:
mimeType: string? mimeType: string?
buffer: binary buffer: binary
localDirectory: string? localDirectory: string?
directoryStream: WritableStream?
localPaths: localPaths:
type: array? type: array?
items: string items: string

View file

@ -80,6 +80,20 @@ it('should upload a folder and throw for multiple directories', async ({ page, s
await expect(input.setInputFiles([ await expect(input.setInputFiles([
path.join(dir, 'folder1'), path.join(dir, 'folder1'),
path.join(dir, 'folder2'), path.join(dir, 'folder2'),
])).rejects.toThrow('Multiple directories are not supported');
});
it('should throw if a directory and files are passed', async ({ page, server, browserName, headless, browserMajorVersion }) => {
await page.goto(server.PREFIX + '/input/folderupload.html');
const input = await page.$('input');
const dir = path.join(it.info().outputDir, 'file-upload-test');
{
await fs.promises.mkdir(path.join(dir, 'folder1'), { recursive: true });
await fs.promises.writeFile(path.join(dir, 'folder1', 'file1.txt'), 'file1 content');
}
await expect(input.setInputFiles([
path.join(dir, 'folder1'),
path.join(dir, 'folder1', 'file1.txt'),
])).rejects.toThrow('File paths must be all files or a single directory'); ])).rejects.toThrow('File paths must be all files or a single directory');
}); });