Proposal: be more strict about WithListener

[costela]

Describe the feature request

I would like to make WithListener more strict about its parameter, requiring the consumer to implement some of the listener interfaces.

The rationale being: it's less work to implement a NOOP interface than it is to debug code that silently doesn't do what it's told.

Background

Currently, when using WithListener(), it is possible to introduce silent errors by not implementing any of the listener interfaces. The sync() goroutine will be started, but none of the listeners will be set.

This is presumably intentional, so that one can turn the sync on without having to actually implement anything (WithListener(struct{}{})), but it is also extremely error-prone.

The following snippet showcases how easy it is to miss this:

func init() {
	unleash.Initialize(
		unleash.WithListener(unleashListener{}),
	)
}

var _ unleash.RepositoryListener = (*unleashListener)(nil)

type unleashListener struct {}

func (*unleashListener) OnReady() {
	log.Print("ready")
}

Note the interface-mismatch: we test against a pointer (*unleashListener) but pass a copy (unleashListener{}).

Solution suggestions

We could trivially check if any listener interface is implemented in NewClient, but that would be a tricky compatibility breakage: old code would compile, but not run.

Maybe it would be better to break the API completely and change ConfigOption into the following?

type ConfigOption func(*configOption) error

With this, we could move a lot of the validation code currently living in NewClient to its respective option.

WDYT?

[sighphyre]

@costela I think it's a fairly large change to break API compatibility for ergonomic reasons. I'm not against this but this is probably not going to be a priority for us for quite a while. Can I tag this as a maybe later issue? If you're willing to expand on this idea a little and do PR I'd be open to reviewing it but it's unlikely we'll merge it until we do a major version release for this SDK, which is probably going to be a while

[costela]

hi @sighphyre! Totally understand the reluctance. I'll cook up a PR and check how much would actually break.

[sighphyre]

You're amazing, thank you @costela!

[thomasheartman]

Hey 🙋 It's been a while now. Is there any progress on this? @costela Did you get anywhere with the PR?