Modify search parameters easily

[bobylito]

Before we needed to rely on using SearchParameters.mutateMe with AlgoliasearchHelper.setState to modify any of the parameters, we can set through the constructor but don't have setters.

Now we can either use one of the more friendly SearchParameters methods : setQueryParameter (to set a single parameter) or setQueryParameters (to set multiple parameters at once)

Even more more friendly, setQueryParameter can be set directly on the helper. This method relies internally on its SearchParameters counterpart, and adds the event handling for more convenience.

[redox]

I would not check if the parameter exists here. We may want to set something custom/hidden or not yet supported by the helper. Is that doable?

[bobylito]

For several reasons, it would not be consistent with the behavior of the SearchParameters and more broadly of the Helper :

• All setters rely on mutateMe which uses the constructor to create a new instance. The constructor is made so it explicitly take the correct parameters, it results that it also filters wrong parameters.
• I think we're better off with a system that explicitly describes the parameters of the queries. There are many of them and it's practical to have them with the code of the client/library we use. If some parameters can be left undocumented, that's not really good.

I'll make sure we keep the parameters in sync.

[redox]

even with something like:

if( k.indexOf( parameter ) === -1 ) {
  this.unknown[parameter] = value;
} else {
  // bla bla
}

Not a big deal right now, let's wait the first time we have the issue to re-think about it :)

[bobylito]

If we ever happen to have a problem with this approach, that would be sad but we would be able to simplify the code. In the mean time, I agree let's try that and see :)

[redox]

I would then throw an exception or something instead of the return; otherwise we'll spend hours to figure out typos. What do you think?

[vvo]

👍 Silently failing will lead to incomprehension.

Throwing a new Error('Unknown query parameter') smg like that seems fine?

[bobylito]

Looks good to me! Should we consider a less destroying solution like logging?

[vvo]

If I was using this I would really want it to throw if I am passing an unknown argument. If the goal is to only accept known arguments then it should throw. Logging would be ok if it was not an error.

But the client will expect something to work and it will not so it seems like it's not a warning.

You could also emit an error and if he does not listen to the error event of an EventEmitter it will throw (default behavior).

But then just throwing seems better to me.

[bobylito]

Sounds right to me then :)

The SearchParameters does not have event emitters so throw everything!! :)

[acechase]

@bobylito - Looks great - I'm looking forward to replacing my mutateMe method calls with this :)

[redox]

LGTM 👍

[bobylito]

Awesome, thanks for the comments and support @redox @vvo @acechase