ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: sdk/src/types/globals.ts (TypeScript)

Code before:
// using "| any" to ensure compatibility with various "Fetch" alternative function signatures
export type FetchInterface = (input: string | any, init?: RequestInit | any) => Promise<Response>;

export type UrlInterface = typeof URL;

/** While the standard says 'string | URL' for the 'url' parameter, some implementations (e.g. React Native) only accept 'string' */
export type WebSocketConstructor = {
	new (url: string, protocols?: string | string[]): WebSocketInterface;
};

export type WebSocketInterface = {
	readonly readyState: number;

	addEventListener(type: string, listener: (this: WebSocketInterface, ev: any) => any): void;
	removeEventListener(type: string, listener: (this: WebSocketInterface, ev: any) => any): void;

	send(data: string): void;
	close(code?: number, reason?: string): void;
};

export type LogLevels = 'log' | 'info' | 'warn' | 'error';

export type ConsoleInterface = {
	[level in LogLevels]: (...args: any) => any;
};


Review comment (suggestion):
Unfortunately this is not something we can do, it must stay as `unknown` to ensure non-native/alternative fetch implementations (e.g. `ofetch`) are supported
```suggestion
export type FetchInterface = (input: string | any, init?: RequestInit | any) => Promise<unknown>;
```

Code after:
// using "| any" to ensure compatibility with various "Fetch" alternative function signatures
export type FetchInterface = (input: string | any, init?: RequestInit | any) => Promise<unknown>;

export type UrlInterface = typeof URL;

/** While the standard says 'string | URL' for the 'url' parameter, some implementations (e.g. React Native) only accept 'string' */
export type WebSocketConstructor = {
	new (url: string, protocols?: string | string[]): WebSocketInterface;
};

export type WebSocketInterface = {
	readonly readyState: number;

	addEventListener(type: string, listener: (this: WebSocketInterface, ev: any) => any): void;
	removeEventListener(type: string, listener: (this: WebSocketInterface, ev: any) => any): void;

	send(data: string): void;
	close(code?: number, reason?: string): void;
};

export type LogLevels = 'log' | 'info' | 'warn' | 'error';

export type ConsoleInterface = {
	[level in LogLevels]: (...args: any) => any;
};
Repository: directus/directus
File: sdk/src/utils/request.ts (TypeScript)

Code before:
import type { FetchInterface, DirectusError } from '../index.js';
import { extractData } from './extract-data.js';

/**
 * Request helper providing default settings
 *
 * @param url The request URL
 * @param options The request options
 *
 * @returns The API result if successful
 */
export const request = async <Output = any>(
	url: string,
	options: RequestInit,
	fetcher: FetchInterface = globalThis.fetch,
): Promise<Output> => {
	options.headers =
		typeof options.headers === 'object' && !Array.isArray(options.headers)
			? (options.headers as Record<string, string>)
			: {};

	return fetcher(url, options).then((response) => {
		return extractData(response).catch((reason) => {
			const result: DirectusError = {
				message: 'Unknown api error',
				errors: reason && typeof reason === 'object' && 'errors' in reason ? reason.errors : reason,
				response,
			};

			if (reason && typeof reason === 'object' && 'data' in reason) result.data = reason.data;

			if (result.errors[0] && result.errors[0].message) {
				// TODO first or last error?
				result.message = result.errors[0].message;
			}

			return Promise.reject(result);
		});
	});
};


Review comment (suggestion):
I believe this should default to an empty string to be inline with the [documented default](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/message#description) 

Code after:
import type { FetchInterface } from '../index.js';
import { extractData } from './extract-data.js';

/**
 * Request helper providing default settings
 *
 * @param url The request URL
 * @param options The request options
 *
 * @returns The API result if successful
 */
export const request = async <Output = any>(
	url: string,
	options: RequestInit,
	fetcher: FetchInterface = globalThis.fetch,
): Promise<Output> => {
	options.headers =
		typeof options.headers === 'object' && !Array.isArray(options.headers)
			? (options.headers as Record<string, string>)
			: {};

	return fetcher(url, options).then((response) => {
		return extractData(response).catch((reason) => {
			const result: { response: unknown; message: string; errors: any; data?: any } = {
				message: '',
				errors: reason && typeof reason === 'object' && 'errors' in reason ? reason.errors : reason,
				response,
			};

			if (reason && typeof reason === 'object' && 'data' in reason) result.data = reason.data;

			if (Array.isArray(result.errors) && result.errors[0]?.message) {
				result.message = result.errors[0].message;
			}

			return Promise.reject(result);
		});
	});
};
Repository: directus/directus
File: app/src/components/v-input.vue (Vue)

Code before:

	&.small {
		block-size: 38px;

		.input {
			padding: 8px 12px;
		}
	}

	&.full-width {
		inline-size: 100%;

		.input {
			inline-size: 100%;
		}
	}

	&.has-click {
		cursor: pointer;

		&.disabled {
			cursor: auto;
		}

		input {
			pointer-events: none;

			&::selection {
				background-color: transparent;
			}

			.prefix,
			.suffix {
				color: var(--theme--foreground-subdued);
			}
		}

		.append-outer {
			margin-inline-start: 8px;
		}
	}

	&.invalid input:not(:focus) {
		text-decoration: line-through;
		color: var(--theme--foreground-subdued);
	}

	.warning-invalid {
		--v-icon-color: var(--theme--warning);

		margin-inline-end: 8px;

Review comment (refactor):
Please re-add the rule, as we need it to prevent the selection of clickable inputs. In the future we will refactor this to a button, but for now let’s keep the rule.

Code after:

	&.small {
		block-size: 38px;

		.input {
			padding: 8px 12px;
		}
	}

	&.full-width {
		inline-size: 100%;

		.input {
			inline-size: 100%;
		}
	}

	&.has-click {
		cursor: pointer;

		&.disabled {
			cursor: auto;
		}

		input {
			pointer-events: none;
			-webkit-user-select: none;
			user-select: none;

			&::selection {
				background-color: transparent;
			}

			.prefix,
			.suffix {
				color: var(--theme--foreground-subdued);
			}
		}

		.append-outer {
			margin-inline-start: 8px;
		}
	}

	&.invalid input:not(:focus) {
		text-decoration: line-through;
		color: var(--theme--foreground-subdued);
	}

	.warning-invalid {
		--v-icon-color: var(--theme--warning);
Repository: directus/directus
File: app/src/operations/error/index.ts (TypeScript)

Code before:
							value: '401',
						},
						{
							text: '403 - Forbidden',
							value: '403',
						},
						{
							text: '404 - Not Found',
							value: '404',
						},
						{
							text: '405 - Method Not Allowed',
							value: '405',
						},
						{
							text: '422 - Unprocessable Entity',
							value: '422',
						},
						{
							text: '429 - Too Many Requests',
							value: '429',
						},
						{
							text: '500 - Internal Server Error',
							value: '500',
						},
					],
					allowOther: true,
				},
			},
		},
		{
			field: 'message',
			name: '$t:operations.error.message',
			type: 'string',
			meta: {
				width: 'full',
				interface: 'input',
			},
		},
	],
});


Review comment (security):
```suggestion
						{
							text: '400 (Bad Request)',
							value: '400',
						},
						{
							text: '401 (Unauthorized)',
							value: '401',
						},
						{
							text: '403 (Forbidden)',
							value: '403',
						},
						{
							text: '404 (Not Found)',
							value: '404',
						},
						{
							text: '405 (Method Not Allowed)',
							value: '405',
						},
						{
							text: '422 (Unprocessable Entity)',
							value: '422',
						},
						{
							text: '429 (Too Many Requests)',
							value: '429',
						},
						{
							text: '500 (Internal Server Error)',
							value: '500',
						},
```

Code after:
							value: '401',
						},
						{
							text: '403 (Forbidden)',
							value: '403',
						},
						{
							text: '404 (Not Found)',
							value: '404',
						},
						{
							text: '405 (Method Not Allowed)',
							value: '405',
						},
						{
							text: '422 (Unprocessable Entity)',
							value: '422',
						},
						{
							text: '429 (Too Many Requests)',
							value: '429',
						},
						{
							text: '500 (Internal Server Error)',
							value: '500',
						},
					],
					allowOther: true,
				},
			},
		},
		{
			field: 'message',
			name: '$t:operations.error.message',
			type: 'string',
			meta: {
				width: 'full',
				interface: 'input',
			},
		},
	],
});
Repository: directus/directus
File: app/src/utils/get-local-type.ts (TypeScript)

Code before:
		return 'standard';
	}

	if (relations.length === 1) {
		const relation = relations[0]!;
		if (relation.related_collection === 'directus_files' && relation.related_collection !== collection) return 'file';
		if (relation.collection === collection && relation.field === field) return 'm2o';
		return 'o2m';
	}

	if (relations.length === 2) {
		if ((fieldInfo.meta?.special || []).includes('translations')) {
			return 'translations';
		}

		if ((fieldInfo.meta?.special || []).includes('m2a')) {
			return 'm2a';
		}

		const relationForCurrent = getRelation(relations, collection, field);

		if (relationForCurrent?.collection === collection && relationForCurrent?.field === field) {
			return 'm2o';
		}

		if ((fieldInfo.meta?.special || []).includes('m2m')) {
			return 'm2m';
		}
	}

	return 'standard';
}


Review comment (suggestion):
We can fix this issue without removing the existing behavior by checking if the root collection is `directus_files` in the condition.

```suggestion
		const directusFilesRelationsCount = relations.filter((relation) => relation.related_collection === 'directus_files').length;

		const isRelationToDirectusFiles = collection !== 'directus_files' && directusFilesRelationsCount === 1;
		const isSelfRelationToDirectusFiles = directusFilesRelationsCount === 2;

		if (isRelationToDirectusFiles || isSelfRelationToDirectusFiles) {
			return 'files';
		} else {
			return 'm2m';
		}
```

Note that I have not tested the example above. Explanation:
We have 2 related collections.
If both related collections are `directus_files`, then we want to return `files` (`isSelfRelationToDirectusFiles`).
If only one related collection is `directus_files`, then we only want to return `files` if the root collection is not `directus_files` (`isRelationToDirectusFiles`).

Code after:
		return 'standard';
	}

	if (relations.length === 1) {
		const relation = relations[0]!;
		if (relation.related_collection === 'directus_files' && relation.related_collection !== collection) return 'file';
		if (relation.collection === collection && relation.field === field) return 'm2o';
		return 'o2m';
	}

	if (relations.length === 2) {
		if ((fieldInfo.meta?.special || []).includes('translations')) {
			return 'translations';
		}

		if ((fieldInfo.meta?.special || []).includes('m2a')) {
			return 'm2a';
		}

		const relationForCurrent = getRelation(relations, collection, field);

		if (relationForCurrent?.collection === collection && relationForCurrent?.field === field) {
			return 'm2o';
		}

		const directusFilesRelationsCount = relations.filter(
			(relation) => relation.related_collection === 'directus_files',
		).length;

		const isRelationToDirectusFiles = collection !== 'directus_files' && directusFilesRelationsCount === 1;
		const isSelfRelationToDirectusFiles = directusFilesRelationsCount === 2;

		if (isRelationToDirectusFiles || isSelfRelationToDirectusFiles) {
			return 'files';
		} else {
			return 'm2m';
		}
	}

	return 'standard';
}
Repository: directus/directus
File: app/src/components/v-form/form-field.vue (Vue)

Code before:

			if (!isEqual(internalValue.value, newVal)) {
				internalValue.value = newVal;
			}
		},
	);

	return { internalValue, isEdited, defaultValue };

	function getInternalValue(): any {
		if (props.modelValue !== undefined) return props.modelValue;
		if (props.initialValue !== undefined) return props.initialValue;
		return defaultValue.value;
	}
}
</script>

<template>
	<div
		:data-primary-key="primaryKey"
		:data-collection="field.collection"
		:data-field="field.field"
		class="field"
		:class="[field.meta?.width || 'full', { invalid: validationError }]"
	>
		<v-menu v-if="!isLabelHidden" :placement="isRTL ? 'bottom-end' : 'bottom-start'" show-arrow arrow-placement="start">
			<template #activator="{ toggle, active }">
				<form-field-label
					:field="field"
					:toggle="toggle"
					:active="active"
					:batch-mode="batchMode"
					:batch-active="batchActive"
					:edited="isEdited"
					:has-error="!!validationError"
					:badge="badge"
					:raw-editor-enabled="rawEditorEnabled"
					:raw-editor-active="rawEditorActive"
					:loading="loading"
					@toggle-batch="$emit('toggle-batch', $event)"
					@toggle-raw="$emit('toggle-raw', $event)"
				/>
			</template>

			<form-field-menu
				:field="field"
				:model-value="internalValue"
				:initial-value="initialValue"
				:restricted="isDisabled"
				:disabled-options="disabledMenuOptions"
				@update:model-value="emitValue($event)"

Review comment (suggestion):
Let’s keep the placement value at `bottom-start` here, since `start` means `left` in LTR and `right` in RTL. Instead, let’s make the change in the `v-menu` component — see the other comment.

Code after:
	);

	return { internalValue, isEdited, defaultValue };

	function getInternalValue(): any {
		if (props.modelValue !== undefined) return props.modelValue;
		if (props.initialValue !== undefined) return props.initialValue;
		return defaultValue.value;
	}
}
</script>

<template>
	<div
		:data-primary-key="primaryKey"
		:data-collection="field.collection"
		:data-field="field.field"
		class="field"
		:class="[field.meta?.width || 'full', { invalid: validationError }]"
	>
		<v-menu v-if="!isLabelHidden" placement="bottom-start" show-arrow arrow-placement="start">
			<template #activator="{ toggle, active }">
				<form-field-label
					:field="field"
					:toggle="toggle"
					:active="active"
					:batch-mode="batchMode"
					:batch-active="batchActive"
					:edited="isEdited"
					:has-error="!!validationError"
					:badge="badge"
					:raw-editor-enabled="rawEditorEnabled"
					:raw-editor-active="rawEditorActive"
					:loading="loading"
					@toggle-batch="$emit('toggle-batch', $event)"
					@toggle-raw="$emit('toggle-raw', $event)"
				/>
			</template>

			<form-field-menu
				:field="field"
				:model-value="internalValue"
				:initial-value="initialValue"
				:restricted="isDisabled"
				:disabled-options="disabledMenuOptions"
				@update:model-value="emitValue($event)"
				@unset="$emit('unset', $event)"
				@edit-raw="showRaw = true"
				@copy-raw="copyRaw"
				@paste-raw="pasteRaw"
			/>
Repository: directus/directus
File: app/src/components/v-menu.vue (Vue)

Code before:
				name: 'applyStyles',
				enabled: true,
				phase: 'write',
				fn({ state }) {
					if (state.styles.popper) styles.value = state.styles.popper;

					if (state.styles.arrow) {
						if (props.arrowPlacement === 'start') {
							let x = 0;
							let y = 0;

							switch (state.placement) {
								case 'top-start':
								case 'bottom-start':
									x = props.arrowPadding;
									break;
								case 'bottom-end':
									x = props.arrowPadding * -1;
									break;
								case 'left-start':
								case 'right-start':
									y = props.arrowPadding;
									break;
							}

							state.styles.arrow.transform = `translate3d(${x}px, ${y}px, 0)`;

							if (isRTL.value) {
								state.styles.arrow.right = state.styles.arrow.left;
								state.styles.arrow.left = `unset`;
							}
						}

						arrowStyles.value = state.styles.arrow;
					}

					callback();
				},
			},
			{
				name: 'placementUpdater',
				enabled: true,
				phase: 'afterWrite',
				fn({ state }) {
					placement.value = state.placement;
				},
			},
		];

		return modifiers;
	}

Review comment (suggestion):
Let’s make the change in the `bottom-start` block and remove the `bottom-end` block. This way it will also work for the `top-start` placement.

Example (not tested):
```ts
case 'top-start':
case 'bottom-start':
	x = props.arrowPadding;
	if (isRTL.value) x *= -1;
	break;
```

Code after:
					});

					if (state.placement.startsWith('top') && overflow.top < 0) {
						forceMaxHeight.value = state.elements.popper.offsetHeight - Math.ceil(overflow.top);
					} else if (state.placement.startsWith('bottom') && overflow.bottom > 0) {
						forceMaxHeight.value = state.elements.popper.offsetHeight - Math.floor(overflow.bottom);
					} else {
						forceMaxHeight.value = null;
					}
				},
			},
			{
				name: 'applyStyles',
				enabled: true,
				phase: 'write',
				fn({ state }) {
					if (state.styles.popper) styles.value = state.styles.popper;

					if (state.styles.arrow) {
						if (props.arrowPlacement === 'start') {
							let x = 0;
							let y = 0;

							switch (state.placement) {
								case 'top-start':
								case 'bottom-start':
									x = props.arrowPadding;
									break;
								case 'top-end':
								case 'bottom-end':
									x = props.arrowPadding * -1;
									break;
								case 'left-start':
								case 'right-start':
									y = props.arrowPadding;
									break;
							}

							state.styles.arrow.transform = `translate3d(${x}px, ${y}px, 0)`;

							if (isRTL.value) {
								state.styles.arrow.right = state.styles.arrow.left;
								state.styles.arrow.left = `unset`;
							}
						}

						arrowStyles.value = state.styles.arrow;
					}

					callback();
				},
Repository: directus/directus
File: app/src/components/v-menu.vue (Vue)

Code before:
	>,
): Record<string, any> {
	const popperInstance = ref<Instance | null>(null);
	const styles = ref({});
	const arrowStyles = ref({});

	// The internal placement can change based on the flip / overflow modifiers
	const placement = ref(options.value.placement);

	onUnmounted(() => {
		stop();
	});

	function checkPlacement() {
		if (isRTL.value && options.value.placement.includes('start')) {
			return options.value.placement.replace(/start/g, 'end');
		}

		return options.value.placement;
	}

	watch(
		options,
		() => {
			popperInstance.value?.setOptions({
				placement: options.value.attached ? 'bottom-start' : (checkPlacement() as Placement),
				modifiers: getModifiers(),
			});
		},
		{ immediate: true },
	);

	const observer = new MutationObserver(() => {
		popperInstance.value?.forceUpdate();
	});

	return { popperInstance, placement, start, stop, styles, arrowStyles };

	function start() {
		return new Promise((resolve) => {
			popperInstance.value = createPopper(reference.value!, popper.value!, {
				placement: options.value.attached ? 'bottom-start' : (checkPlacement() as Placement),
				modifiers: getModifiers(resolve),
				strategy: 'fixed',
			});

			popperInstance.value.forceUpdate();

			observer.observe(popper.value!, {
				attributes: false,
				childList: true,

Review comment (suggestion):
Let’s include the `options.value.attached ? 'bottom-start'` condition in the `checkPlacement` function, since we should also reverse `bottom-start` to `bottom-end` when in RTL mode (when `options.value.attached` is `true`). In addition, it might make sense to rename `checkPlacement` to `getPlacement`.

Code after:
	>,
): Record<string, any> {
	const popperInstance = ref<Instance | null>(null);
	const styles = ref({});
	const arrowStyles = ref({});

	// The internal placement can change based on the flip / overflow modifiers
	const placement = ref(options.value.placement);

	onUnmounted(() => {
		stop();
	});

	function getPlacement() {
		if (isRTL.value) {
			if (options.value.attached) {
				return 'bottom-end';
			} else if (options.value.placement.includes('start') || options.value.placement.includes('end')) {
				return options.value.placement.replace(/start|end/g, (match) => (match === 'start' ? 'end' : 'start'));
			}
		}

		return options.value.attached ? 'bottom-start' : options.value.placement;
	}

	watch(
		options,
		() => {
			popperInstance.value?.setOptions({
				placement: getPlacement() as Placement,
				modifiers: getModifiers(),
			});
		},
		{ immediate: true },
	);

	const observer = new MutationObserver(() => {
		popperInstance.value?.forceUpdate();
	});

	return { popperInstance, placement, start, stop, styles, arrowStyles };

	function start() {
		return new Promise((resolve) => {
			popperInstance.value = createPopper(reference.value!, popper.value!, {
				placement: getPlacement() as Placement,
				modifiers: getModifiers(resolve),
				strategy: 'fixed',
			});

			popperInstance.value.forceUpdate();
Repository: directus/directus
File: app/src/components/v-menu.vue (Vue)

Code before:
					}
				},
			},
			{
				name: 'applyStyles',
				enabled: true,
				phase: 'write',
				fn({ state }) {
					if (state.styles.popper) styles.value = state.styles.popper;

					if (state.styles.arrow) {
						if (props.arrowPlacement === 'start') {
							let x = 0;
							let y = 0;

							switch (state.placement) {
								case 'top-start':
								case 'bottom-start':
									x = props.arrowPadding;
									break;
								case 'bottom-end':
									x = props.arrowPadding * -1;
									break;
								case 'left-start':
								case 'right-start':
									y = props.arrowPadding;
									break;
							}

							state.styles.arrow.transform = `translate3d(${x}px, ${y}px, 0)`;

							if (isRTL.value) {
								state.styles.arrow.right = state.styles.arrow.left;
								state.styles.arrow.left = `unset`;
							}
						}

						arrowStyles.value = state.styles.arrow;
					}

					callback();
				},
			},
			{
				name: 'placementUpdater',
				enabled: true,
				phase: 'afterWrite',
				fn({ state }) {
					placement.value = state.placement;
				},
			},

Review comment (suggestion):
Let’s also apply this to `top-end`.

```suggestion
								case 'top-end':
								case 'bottom-end':
```

However, let’s make sure to test all these cases in LTR and RTL.

Code after:
					} else if (state.placement.startsWith('bottom') && overflow.bottom > 0) {
						forceMaxHeight.value = state.elements.popper.offsetHeight - Math.floor(overflow.bottom);
					} else {
						forceMaxHeight.value = null;
					}
				},
			},
			{
				name: 'applyStyles',
				enabled: true,
				phase: 'write',
				fn({ state }) {
					if (state.styles.popper) styles.value = state.styles.popper;

					if (state.styles.arrow) {
						if (props.arrowPlacement === 'start') {
							let x = 0;
							let y = 0;

							switch (state.placement) {
								case 'top-start':
								case 'bottom-start':
									x = props.arrowPadding;
									break;
								case 'top-end':
								case 'bottom-end':
									x = props.arrowPadding * -1;
									break;
								case 'left-start':
								case 'right-start':
									y = props.arrowPadding;
									break;
							}

							state.styles.arrow.transform = `translate3d(${x}px, ${y}px, 0)`;

							if (isRTL.value) {
								state.styles.arrow.right = state.styles.arrow.left;
								state.styles.arrow.left = `unset`;
							}
						}

						arrowStyles.value = state.styles.arrow;
					}

					callback();
				},
			},
			{
				name: 'placementUpdater',
				enabled: true,