Add a generic form of `Selection.Map` (requiring a more recent Go version)

[Fesaa]

Heya!

Was playing around with this in a project, and it has been amazing!

Noticed the Selection.Map only allowing to return a string, so wanted to change that to generics so it could return anything as currently I've used Each every time (and manually putting it in the array), while Map would be nicer to use.

As such the change;

func (s *Selection) Map[T any](f func(int, *Selection) T) (result []T) {
	for i, n := range s.Nodes {
		result = append(result, f(i, newSingleSelection(n, s.document)))
	}

	return result
}

Noticed the project is still on 1.13 when trying to add it myself, so was wondering if there are any plans to update to a newer version to allowed to usage of generics, and other later introduced features.

Thanks for the amazing project! 💕

[mna]

Hello Amelia,

Thanks for the kind words! That's a great idea to move that Map function to a generic type. I wonder if we could follow a precedent from the stdlib regarding how to introduce this, e.g. it could be a new Selection.MapT[T any](...) instead of making a potential breaking change (although in many cases, the code should still work as-is with type inference).

I tried looking at recent release changes notes but the only thing I've seen so far is rand.N as a generic form of IntN/Int32n/etc.. There's also reflect.TypeFor[T any] but that one followed a more natural naming based on what it does that is hard to apply outside reflect.

Will keep taking a look, happy to hear your thoughts on this too. I think the proposal itself (a generic form of Map) is definitely a good thing to add, just wondering how to go about it for the naming. I think I'd prefer to err on the side of caution and consider that a breaking change (if we reuse the Map method name), and in that sense try to avoid doing this. I don't think that requiring a more recent Go version for goquery is an issue, it would be released as a new minor version (if using a new method name), and as such, I'd document that starting with this version Go 1.18 (or whatever it ends up being) is now required.

Thanks, Martin

[Fesaa]

Heya!

The stdlib introduced a lot of generic functions with the slices package, like;

func IndexFunc[S ~[]E, E any](s S, f func(E) bool) int {
	for i := range s {
		if f(s[i]) {
			return i
		}
	}
	return -1
}

However, it should be fine in this case to do as I wrote above, as the slice is not passed as a function argument. Interesting blog about why the type arguments are like this; https://go.dev/blog/deconstructing-type-parameters!

(Un)fortunately, go doesn't allow a method to have generics, without the struct having any. So the generic function would have to be introduced as a standalone function. On the flip side, this does ensure no breaking changes!

[mna]

Hey!

The stdlib introduced a lot of generic functions with the slices package

Yeah, sorry I should've been clearer, I meant examples of existing functions that received a generic version later on, and how they handled the naming of that generic one. But that's all moot anyway because...

However, it should be fine in this case to do as I wrote above, as the slice is not passed as a function argument. So the generic function would have to be introduced as a standalone function.

So you mean that instead of having the generic Map as a method on Selection (like the example you added in your first comment), you'd propose having a new generic Map function as a package-level function, like this?

func Map[T any](s *Selection, f func(int, *Selection) T) (result []T) {
	for i, n := range s.Nodes {
		result = append(result, f(i, newSingleSelection(n, s.document)))
	}
	return result
}

If so, I totally agree, that sounds good and introduces no breaking change concerns. Would you like to provide a PR for that? Happy to get that merged if you do, just a few things to include beyond the func itself: some helpful doc comment (probably similar to the Map method), some tests for the new func and updating the go.mod Go version (if you wish, I can also do it as part of cutting the new release).

Thanks! Martin

[mna]

Closed by #467