Hits widget transformItems is called twice, receives already-transformed items

[QuakeMatt]

🐛 Bug description

It appears that the transformItems function of the hits widget is being called twice for a single search invocation, and is receiving the already-transformed items on the second run.

🔍 Bug reproduction

Steps to reproduce the behavior:

1. Add a transformItems callback to the hits widget that returns transformed items.
2. Add a console.log notification somewhere inside the transformation function.
3. Reload the page and look for notifications in the console.

Live reproduction:

https://codesandbox.io/s/fervent-frost-bbzb5?file=/src/app.js
https://codesandbox.io/s/stupefied-frog-c2krg?file=/src/instantsearch.ts

Including two examples here, one based on create-instantsearch-app and another based on the instantsearch sandbox from the main documentation. The latter already included a transformItems call, so required fewer additions on my part.

💭 Expected behavior

transformItems is called only once for a single search invocation, and is never passed already-transformed items.

Environment

• OS: Windows
• Browser: Firefox, Edge, Chrome
• Version: 4.25.2

Additional context

I've run into this bug while upgrading from an older 4.3.1 install to the latest 4.25.2 release. However, experimenting via codesandbox.io reveals that the bug was likely introduced in the 4.9 branch, as 4.9.2 is the earliest release I tested that allowed me to replicate it.

It particularly affects us because our transformed items cannot be safely transformed a second time, leading to an unexpected runtime error rather than it silently continuing.

Possibly related to this issue in the vue-instantsearch repo?

[tkrugg]

Hi, thanks for reaching out.
This is a behaviour we are aware of. It was indeed introduced in #4525.
It's related to the fact we call twice in the render phase: once in getRenderState to compute the hits commited to the state, and another in render. This brings some consistency in the way we handle the state in various connectors.

@algolia/instantsearch-for-websites I think a lot of users expect transformItems to apply only to the widget/connector they were defined on, so I'm wondering is we can make an exception on transformItems to have them computed only when executing render?

[QuakeMatt]

Ah, apologies, I did have a look through the issues, but couldn't find anything that seemed to match.

For my use-case the doubling up of the transformation is the bigger issue. If it turns out it's necessary for the transformer to be called twice, would it be feasible for it to be passed the original, un-transformed hits array in both instances?

[tkrugg]

Yes that would be a sensible behaviour, I'd need some time to look at the implications this change would have.
In the meantime a workaround I can suggest one of the following:

• use a memoization mechanism, try picking one using an LRU cache of size 1 so you don't unecessarily store all the results,
• use the connector's API:

// instead of 
const Hits = instantsearch.connectors.hits({
     contrainer: "#hits",
     transformItems: hits => myTransform(hits)
})


// do this

const Hits = instantsearch.connectors.connectHits((renderOptions, isFirstRender) => {
  const { hits } = renderOptions;

  const _hits = myTransform(hits);

  document.querySelector('#hits').innerHTML = `
    <ul>
      ${hits
        .map(
          item =>
            `<li>
              ${instantsearch.highlight({ attribute: 'name', hit: item })}
            </li>`
        )
        .join('')}
    </ul>
  `;
});

see our docs for more examples: https://www.algolia.com/doc/api-reference/widgets/hits/js/#connector-param-render-hits

[Haroenv]

Yes, it would be interesting to hear why there's a problem if your transformItems gets called multiple times. Which transformation do you do in it?

[QuakeMatt]

In the meantime a workaround I can suggest one of the following

Thanks, I'll take a look.

Yes, it would be interesting to hear why there's a problem if your transformItems gets called multiple times. Which transformation do you do in it?

In our case it's a product object that gets transformed into a more view-friendly format, including image path concatenation and a price conversion into the local currency. Here's a simplfied example:

function transformItem(product) {
    return {
        name: product.name,
        image: THUMBNAIL_PATH + product.image,
        price: toLocalCurrency(product.basePrice.gross), // e.g. 12.5 > "€13,73"
    };
}

When the second transformation call is made, it's passed the already-transformed items and raises an error because the basePrice property doesn't exist in the new output. I could technically add it to avoid the hard error, but that'll still lead to problems with e.g. the image path being doubled up.

[Haroenv]

I see how that's problematic, for the time being I can only think of in transformItems adding new keys you plan to display instead of actually changing existing values.

Not sure yet how, but I think we probably should avoid this as it could be considered a bug. I think it happens because we set the result of transformItems to searchResults, for mostly legacy reasons (not escaping multiple times)

[QuakeMatt]

I see how that's problematic, for the time being I can only think of in transformItems adding new keys you plan to display instead of actually changing existing values.

No worries - I've got a few different workarounds that I can try, now that I have a better understanding of what's going on.

Thanks for all your help!