Skip to content

Conversation

@gggritso
Copy link
Member

This is an optimization for the SearchQueryBuilder component. Here's the issue I'm seeing:

  • SearchQueryBuilder is rendered (or re-rendered, whichever) with unstable props (because the props are hard to stabilize, which happens sometimes, or why rendering for the first time)
  • The render is fairly slow, it can take anywhere in the 50-100ms time range
  • One of the slower parts is rendering SearchQueryBuilderComboBox. This takes a long time to render because it uses React Stately's useComboBoxState. This hook accepts the children props, which is the collection renderer. It renders all the items at this point, even if the combo box is not open!

So, the entire collection is rendered even if it's not open. This PR does a trick in which if we know the combo box is not open, it provides a null children, so the extra work doesn't have to be done. This saves ~30ms per free text token in the builder, which can be a lot, if there are many tokens, since there's free text around each one.

As far as I can tell, this doesn't cause any adverse effects on the UI.

Before:
Screenshot 2025-11-19 at 4 50 55 PM

After:
Screenshot 2025-11-19 at 4 56 02 PM

@gggritso gggritso requested a review from malwilley November 19, 2025 22:08
@linear
Copy link

linear bot commented Nov 19, 2025

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Nov 19, 2025
Copy link
Member

@malwilley malwilley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find! I wish React Stately would lazily generate the items itself, but it looks like this is a good solution. I've tested and it doesn't look like it works any differently, so looks good to merge 👍

Copy link
Member

@malwilley malwilley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, meant to approve!

@gggritso gggritso merged commit aeb204a into master Nov 20, 2025
47 checks passed
@gggritso gggritso deleted the georgegritsouk/dain-1097-prevent-rendering-closed-combo-box-contents branch November 20, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants