Clarifying the signature of the onMove callback

[stof]

Custom

The doc for options in the readme describes 3 return values for onMove corresponding to the special behaviors (false, 1 and -1): https://github.com/SortableJS/Sortable/tree/88838bfa5647e278f4cf87fdd4c3e34cd24fb33c#options
Reading the source code at

Sortable/src/Sortable.js

Lines 1247 to 1254 in 88838bf

	after = direction === 1;
	let moveVector = onMove(rootEl, el, dragEl, dragRect, target, targetRect, evt, after);
	if (moveVector !== false) {
	if (moveVector === 1 || moveVector === -1) {
	after = (moveVector === 1);
	}

tells me that there is a 4th behavior: for any other returned value (for instance when you don't return anything), the library manages the insertion position automatically based on the direction (exactly as done when you don't specify a onMove callback).

I opened a PR at DefinitelyTyped/DefinitelyTyped#45684 to update the type definitions to change the return type to false | 1 | -1 | void, to account for this behavior (so that the typechecker does not complain when returning nothing). but the maintainers answered saying they prefer to stick to the documented API.
Is this behavior when returning nothing an official one ? Suggested answers for preventing the move to certain positions without forcing the insertion side are relying on this, either returning nothing or returning true depending on the snippets (see #1019 and #1500 for such examples).

What should be the official signature for this callback ?

1. false | 1 | -1 (current type definition, preventing this behavior)
2. false | 1 | -1 | void allowing to return nothing to keep the automatic insertion position based on the direction
3. boolean | 1 | -1 | void allowing to return either true or nothing to keep the automatic insertion position

Based on the decision, it would be great to update the readme to reflect that in the doc (I can submit a PR once I know which variant the maintainers want)

[waynevanson]

We need to update the docs to reflect that the signature should return false | 1 | -1 | void.

[stof]

@waynevanson the existing types at @types/sortablejs are documenting boolean rather than only false, making true a valid return value to keep the automatic behavior. Removing support for true there would be a BC break for typescript users.
And supporting true also makes it convenient to write code depending on the kind of conditions you want (as you can return a boolean expression easily).
So should I go for false or boolean in my PR for DefinitelyTyped ? My own vote would go for allowing boolean here, but I don't really have a voice in the final decision, as I'm only a user of the library, not a maintainer or an active contributor.

[waynevanson]

@stof that's interesting point. So returning true uses the insertion point automatically generated?

If this is the case, option three looks to be the correct return value.

[stof]

In the current implementation, returning anything use the automatic insertion point, except for the 3 special values (false, 1 and -1). you could be returning weird things if you want, but I don't think documenting any as the return type makes sense (it would make it too easy to hide mistakes where just some values are special). But true looks like a sensible value to accept.