createEntry maybe not need passed value

[Mini-ghost]

Maybe we not need pass value for createEntry().

In useFieldArray(), function that createEntry() help us create an entry that reference specific path field value with computed.

vee-validate/packages/vee-validate/src/useFieldArray.ts

Lines 58 to 74 in 800641f

	function createEntry(value: TValue): FieldEntry<TValue> {
	const key = entryCounter++;
	const entry: FieldEntry<TValue> = {
	key,
	value: computed<TValue>(() => {
	const currentValues = getFromPath<TValue[]>(form?.values, unref(arrayPath), []);
	const idx = fields.value.findIndex(e => e.key === key);
	return idx === -1 ? value : currentValues[idx];
	}) as any, // will be auto unwrapped
	isFirst: false,
	isLast: false,
	};
	return entry;
	}

vee-validate/packages/vee-validate/src/useFieldArray.ts

Lines 91 to 105 in 800641f

	function push(value: TValue) {
	const pathName = unref(arrayPath);
	const pathValue = getFromPath<TValue[]>(form?.values, pathName);
	const normalizedPathValue = isNullOrUndefined(pathValue) ? [] : pathValue;
	if (!Array.isArray(normalizedPathValue)) {
	return;
	}
	const newValue = [...normalizedPathValue];
	newValue.push(value);
	form?.stageInitialValue(pathName + `[${newValue.length - 1}]`, value);
	form?.setFieldValue(pathName, newValue);
	fields.value.push(createEntry(value));
	updateEntryFlags();
	}

Currently, it will return we passed value when key is not exist in fields, but maybe this will never happen.

As we know computed is evaluated lazily on access, that means it is not computed immediately, until it is accessed.

So, when it is accessed, this entry already exist in filed, the idx is never equal to -1, and we don't need to pass value for createEntry()

On the other hand, if the value is never used, it may still be referenced and let memory leak, so we need to avoid to use the value in computed (in my opinion)

[logaretm]

the idx is never equal to -1

I always use an exhaustive approach when it comes to types and handling. Since findIndex can return -1 then it must be handled. Otherwise, it will cause unexpected outcomes. I agree it is rare but could still happen.

For example, the user passes a field entry to a function, then removes it from the array. Then sets/gets its value from the passed object, then it would be -1 because it no longer exists.

it may still be referenced and let memory leak

Do you have evidence of this? for example, a memory profile showing that it causes a memory leak? If it is never accessed then it shouldn't be a problem.

What issue are you solving here? did you encounter a problem with useFieldArray that you are trying to fix?

[Mini-ghost]

For example, the user passes a field entry to a function, then removes it from the array. Then sets/gets its value from the passed object, then it would be -1 because it no longer exists.

I didn't expect this situation. In this case, should we return undefined when access and let set is invalid, event type is not undefined ?

Do you have evidence of this? for example, a memory profile showing that it causes a memory leak? If it is never accessed then it shouldn't be a problem.

I have no evidence to support this assumption. This is just my inference (or my thinking is wrong), since findIndex may return -1, the value is possible unless the entry is deleted.

Thanks for your comment , I can try to investigate if memory leaks exist if needed .

[Mini-ghost]

Sorry, that my fault. That should be kind of memory wasting instead of memory leak.

---

I found this test in #3618, which made me understand the situation. Maybe it's not memory wasting either, it's intentional

vee-validate/packages/vee-validate/tests/useFieldArray.spec.ts

Lines 33 to 59 in c3c40e5

	test('warns when updating a no-longer existing item', async () => {
	const spy = jest.spyOn(console, 'warn').mockImplementation();
	mountWithHoc({
	setup() {
	useForm({
	initialValues: {
	users: ['1'],
	},
	});
	const { remove, fields } = useFieldArray('users');
	onMounted(() => {
	const item = fields.value[0];
	remove(0);
	item.value = 'test';
	});
	},
	template: `
	<div></div>
	`,
	});
	await flushPromises();
	expect(spy).toHaveBeenCalled();
	spy.mockRestore();
	});

[logaretm]

Yep, also keep in mind that the value utility is important here because you are most likely going to display the item you are iterating on. Furthermore, in v4.6 you will be able to bind it with v-model to make building such fields easier without the Field or useField.