Forward widgetParams to the unmountFn of connectors

[iam4x]

If we save some data, like the container node of our custom widget into widgetParams it would be great to have it with as param of the unmountFn 👍

[francoischalifour]

Hey @iam4x! Do you remember in which cases you needed to access the container node in the connector?

Usually we specify the unmountFn function body in the widget, where we have the knowledge of the container node and unmounts its component.

https://github.com/algolia/instantsearch.js/blob/9dcbdd582d082acaea0edeabdc7e5d6cd9173c51/src/widgets/menu/menu.js#L168-L170

To my knowledge, the connector nevers needs to access the container node because it only calls the unmountFn given by the widget:

https://github.com/algolia/instantsearch.js/blob/9dcbdd582d082acaea0edeabdc7e5d6cd9173c51/src/connectors/menu/connectMenu.js#L228

[Haroenv]

I think the original idea here is that if you give a container to the renderfn & make elements. If you don't particularly save it on the instance, you don't have access to that selector to unmount

[samouss]

We could provide the widgetParams, at least users have access to some information. The use case is more about users that want to cleanup their DOM rather than our own usage. The fact that we rely on a container node is a detail of our implementation. With the widgetParams provided to the function users could do:

const makeHits = instantsearch.connectors.connectHits(
  function renderHits({ hits, widgetParams }) {
    $(widgetParams.container).html(hits.map(hit => JSON.stringify(hit)));
  },
  function unmount({ widgetParams }) {
    $(widgetParams.container).remove();
  }
);

search.addWidget(
  makeHits({
    container: ".hits"
  })
);

[francoischalifour]

I don't think it makes even sense to have the unmount function being exposed as another argument of the connector. I believe we should rather see the render function as a side effect in vanilla JavaScript (because manipulating the DOM is a side effect), and then we should clean up the effect in a function returned by the renderer.

Given your example above @samouss, that would look like:

const makeHits = instantsearch.connectors.connectHits(
  ({ hits, widgetParams }) => {
    $(widgetParams.container).html(hits.map(hit => JSON.stringify(hit)));

    return () => {
      $(widgetParams.container).remove();
    };
  }
);

search.addWidget(
  makeHits({
    container: '.hits',
  })
);

It makes even more sense when you start attaching events to avoid memory leaks. This assumes that we get rid of the init hook first.

I've been thinking a lot about our render API lately and I think I've got some interesting ideas that I'll share in multiple RFCs.