fix(highlight): avoid to ignore highlightedTagName

[samouss]

Summary

With the current implementation we update the source of the highlight two times:

• the first time is inside the connector where we call escapeHits only when escapeHTML is provided. This operation replace the placeholder __ais-highlight__ with the tag mark recursively on all the hits.
• the second time is either in user-land or inside the default template with the highlight or snippet function (though the inline template or a function template). This operation replace the tag em with the provided highlightedTagName. The issue is at this point we don't have a tag em anymore because we replace it the first time with mark we escape the value.

It means that the replace inside highlight & snippet is never executed. This PR aims to fix this issue. We now have two variables:

• TAG_PLACEHOLDER: previously called tagConfig (renamed for consistency)
• TAG_REPLACEMENT: avoid to rely on string and forget to update them

Before

After

---

Alternatives

The current implementation (with or without) the fix supports both syntax the inline Mustache template form with the three braces (because we do the replacement inside the connector):

{{{ _highlightResult.name.value }}}

And of course we also supports the one with the helper:

{{#helpers.highlight}}{ "attribute": "name" }{{/helpers.highlight}}

We recommend the usage of the helper but we perform useless work to support both implementation at the same time. We first transform the placeholder to mark and then we transform mark to the correct tag with the class. We might want to drop the support for the first form. One simple reason is that users don't have to care about Algolia internals.

It simplify the flow of the highlighting: we only have to replace the value inside the helper (we still have to escape the value) it's easier to follow and less error prone. One downside though is that we have to update the refinement list items data structure to allow highlight to work on it. WDYT?

[algobot]

Deploy preview for instantsearchjs ready!

Built with commit f93ba17

https://deploy-preview-3323--instantsearchjs.netlify.com

[Haroenv]

Does this mean that now the {{{ form will not work?

[samouss]

Does this mean that now the {{{ form will not work?

Right now it works because we still do the replace at the same time we escape. But in case we update the implementation to the proposal I made: no it won't work anymore. The value inside the record will still be the placeholder. Only the highlight function will have the knowledge to do the replacement (without mutating the original value).

[tkrugg]

Looks good to me.
I wonder if this has any implications on other flavours:
For example in vue-instantsearch sanitize function: https://github.com/algolia/vue-instantsearch/blob/fbb719616a03092489cb7422f108a1282ae081a2/src/sanitize-results.js

[samouss]

@tkrugg the implication for the other flavour are minimal because they only use the public API. Once the v3 is released we can drop those lines (the ones you linked are for the v1, but the lib doesn't use InstantSearch):

https://github.com/algolia/vue-instantsearch/blob/b4948323b5740df541265db814e7f7c3109c9a23/src/components/Highlight.vue#L32-L51

[Haroenv]

wording suggestion