Bug: selectedLines={null} not respected after line selection event

[jonatanklosko]

Prerequisites

• I have searched for duplicate or closed issues
• I have read the contributing guidelines

Describe the issue

Let's say we pass hard-coded <FileDiff selectedLines={null} ... />. Then we select lines (possibly using the gutter + utility). After the selection finishes, it stays in place, even if we keep passing selectedLines={null}.

Now, the selection stays in place until the component re-renders for any other reason (some other state changing), at that point the selection is cleared.

I believe the issue is that here

pierre/packages/diffs/src/react/utils/useFileDiffInstance.ts

Lines 104 to 120 in d7db28e

	useIsometricEffect(() => {
	const { current: instance } = instanceRef;
	if (instance == null) return;
	const newOptions = mergeFileDiffOptions(options, hasGutterRenderUtility);
	const forceRender = !areOptionsEqual(instance.options, newOptions);
	instance.setOptions(newOptions);
	void instance.render({
	forceRender,
	fileDiff,
	oldFile,
	newFile,
	lineAnnotations,
	});
	if (selectedLines !== undefined) {
	instance.setSelectedLines(selectedLines);
	}
	});

instance.setSelectedLines is called on ever re-render. However, the selection even by itself does not cause a re-render. That also explains why a later re-render does clear the selection.

It probably makes sense to keep the selection if no selectedLines prop is passed at all, similar to uncontrolled React inputs. However, when selectedLines is passed, I would expect it to be the source of truth, similar to controlled React inputs.

Reproduction

As described above.

What browser(s) are you seeing the problem on?

No response

What version of @pierre/diffs are you using?

1.1.4

[amadeus]

Yeah this one is a bit annoying, i'll need to think about it a bit more... selections were not totally designed to be controlled. The api that exposes them is more for setting random selections programmatically, not for managing the internal drag state as you are dragging.

Part of it was because selections aren't really tied to renders (which can be expensive), but a layer on top. InteractionManager sort of maintains the active selected state as you're selecting things (so it nows how to manage like anchors as you drag). This makes it complicated if it becomes controlled, because now we have to manage an internal state for what you are selecting, but then not ACTUALLY update the selection.

can you speak more to what you're trying to do with controlled selections?

I am planning on working on a new decorations api, where you could highlight lines specifically, which might be more useful for what you're trying to accomplish?

There might be some stuff i can clean up with how things are fired that might improve things with the race conditions that happen here

[jonatanklosko]

To give more context, currently I want to use the gutter + for selecting a range of lines, but as soon as the mouse is released I want the selection to disappear (otherwise it just stays in place unless the user selects something else).

[amadeus]

I imagine you could throw a set state in a request animation frame or set timeout to maybe get around this?

I’ll keep thinking about it and tinkering tho

[jonatanklosko]

For sure, forcing a re-render by dummy state update works as a workaround. Definitely not a blocker :)