feat(middleware): accept partial methods

[eunjae-lee]

Summary

This PR allows middlewares to have partial methods. A middleware had to contain all of subscribe, unsubscribe, onStateChange methods, which isn't necessary.

Result

It works the same as before except that the following is possible now:

search.use(() => ({
  onStateChange({ uiState }) {
    // ...
  }
});

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b566664:

Sandbox	Source
InstantSearch.js	Configuration

[Haroenv]

I think this makes sense, but I remember that @francoischalifour originally wanted middlewares to not have optional keys. I wonder if this has changed?

[shortcuts]

Make sense!

[francoischalifour]

I think this makes sense, but I remember that @francoischalifour originally wanted middlewares to not have optional keys. I wonder if this has changed?

The original reason was that going from optional to required is a breaking change, while going the opposite isn't.

Besides, I tend to avoid fully optional structures—now a middleware can be an empty object.

[eunjae-lee]

The original reason was that going from optional to required is a breaking change, while going the opposite isn't.

→ makes sense

Besides, I tend to avoid fully optional structures—now a middleware can be an empty object.

→ do we want to avoid it?

[eunjae-lee]

@francoischalifour

[francoischalifour]

→ do we want to avoid it?

That's not easy to answer. It'll add overhead to both our code and the middleware usage with conditional requirements. We have the same problem with the widget definition where nothing is required, but you still have to provide either init or render. You can't type-check that. That's why I don't think that this PR is necessary personally.

[eunjae-lee]

That's not easy to answer. It'll add overhead to both our code and the middleware usage with conditional requirements. We have the same problem with the widget definition where nothing is required, but you still have to provide either init or render. You can't type-check that. That's why I don't think that this PR is necessary personally.

This PR adds noop upfront, so I don't see it as an overhead. We can show a warning message if it's an empty object. And if we want, we can do it like this, although it's not the cleanest.

[Haroenv]

https://www.typescriptlang.org/play?#code/C4TwDgpgBAsglgEwQGwgdwIYCcIBEIBmcAdnMHAPbFQC8UA3gFBRRUDKwGwEAwgBYZiAcwgAKCmHJUAzgC4ogkAEp5ANwqIA3MyjSArgCNpAYyxwDYlVHVade4vqOnzltRoTaAvtsahIUAEFgABkIDGlgAHliCAAeABUAGigAVVoGAG0AaSgSKABrCBAKAih4gF15AAU4Y3yE5KyAPihPFroq7HIMZASWgDJUjMLi0pTyxl9waE6sbuR4JGRMHHwiUilqOiDQ8KiY2MWUdGw8QhIySmImyeMZYAVqrrgeo+XTtYvN9PpvW-uoAYnnMXgtECgVmd1pcqD8dI4TGYLKIlAxWoxPP8HA9jMD5m9IZ8Nlc4Sx7AjnMjUfR0ZjGHdsVAEHjQQSPudibC6EwWOxONx+IIRCi0Zi6QyIlAICzXuD3qsOTCtgx4YZES4RTTPIk7A41ZTLKKdbziBwuLwBMJDVqMUA is another option to allow partial but not empty objects (source: https://stackoverflow.com/a/48244432/3185307)

[eunjae-lee]

https://www.typescriptlang.org/play?#code/C4TwDgpgBAsglgEwQGwgdwIYCcIBEIBmcAdnMHAPbFQC8UA3gFBRRUDKwGwEAwgBYZiAcwgAKCmHJUAzgC4ogkAEp5ANwqIA3MyjSArgCNpAYyxwDYlVHVade4vqOnzltRoTaAvtsahIUAEFgABkIDGlgAHliCAAeABUAGigAVVoGAG0AaSgSKABrCBAKAih4gF15AAU4Y3yE5KyAPihPFroq7HIMZASWgDJUjMLi0pTyxl9waE6sbuR4JGRMHHwiUilqOiDQ8KiY2MWUdGw8QhIySmImyeMZYAVqrrgeo+XTtYvN9PpvW-uoAYnnMXgtECgVmd1pcqD8dI4TGYLKIlAxWoxPP8HA9jMD5m9IZ8Nlc4Sx7AjnMjUfR0ZjGHdsVAEHjQQSPudibC6EwWOxONx+IIRCi0Zi6QyIlAICzXuD3qsOTCtgx4YZES4RTTPIk7A41ZTLKKdbziBwuLwBMJDVqMUA is another option to allow partial but not empty objects (source: https://stackoverflow.com/a/48244432/3185307)

That's a nice way!

[Haroenv]

I personally think with the "at least one", the downside of empty objects is alleviated, as well as it being instantiated with noops, so no new conditions need to be added. However we need to have a consensus on this as a team

[eunjae-lee]

I personally think with the "at least one", the downside of empty objects is alleviated, as well as it being instantiated with noops, so no new conditions need to be added. However we need to have a consensus on this as a team

What do you mean by "no new conditions"?

[Haroenv]

conditions when using middleware, as it was needed in the initial version of the pr