react-dropdown-tree-select icon indicating copy to clipboard operation
react-dropdown-tree-select copied to clipboard

feat: Do not incur cost of filtering tree if search mode is off

Open gandhis1 opened this issue 3 years ago • 5 comments

What does it do?

When you have tens of thousands of items in a search tree, and you type search input, the cost can be extremely high when you have zero or one characters, especially when you are using a custom search predicate. There is no need to incur the cost of filtering the tree if the text input is zero length. Instead, just reset the search state.

I'll note here that the example in the demo that is supposed to test a large-scale scenario doesn't appear to be all that taxing because the number of nested nodes is very low. You need a lot of nested nodes to encounter performance issues. In my testing, I have almost 100k nodes at up to 10 levels of nesting. I also have keepTreeOnSearch on which probably exacerbates the issue.

This is the first change I would like to do. In a future PR, my intention would be to set the threshold (search term length) at which the search activates. When your search term is "a" you can trigger rendering of thousands of nodes potentially. So this parameter I am envisioning would basically prevent search mode until X characters...with some kind of visual indicator.

Type of change

Please delete options that are not relevant.

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

Checklist:

  • [X] I have performed a self-review of my own code
  • [X] have commented my code, particularly in hard-to-understand areas
  • [X] Updated documentation (if applicable)
  • [ ] Added tests that prove my fix is effective or that my feature works
  • [ ] New and existing unit tests pass locally with my changes
  • [X] My changes generate no new warnings

gandhis1 avatar Jul 13 '22 15:07 gandhis1

Tagging @mrchief

gandhis1 avatar Jul 17 '22 17:07 gandhis1

As a temporary step, I ended up forking this package and publishing my own: https://www.npmjs.com/package/@gandhis1/react-dropdown-tree-select

I then tested it in my main application and this fix works exactly as intended. 100k nodes across 10+ levels does not lock up when you have an empty string search term.

gandhis1 avatar Jul 27 '22 02:07 gandhis1

@gandhis1 Thanks for sending this! This sounds like a great idea and I'll look into this soon. Been swamped at work lately

mrchief avatar Jul 27 '22 20:07 mrchief

Hi any update on this? It's a pretty tiny PR

gandhis1 avatar Aug 06 '22 16:08 gandhis1

Sorry @gandhis1 Will do by end of the week

mrchief avatar Aug 18 '22 16:08 mrchief

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Oct 02 '22 22:10 github-actions[bot]

Following up, can this be reviewed?

gandhis1 avatar Oct 02 '22 23:10 gandhis1

@gandhis1 I'm swamped at the moment but I plan on getting to this, along with the other stuff that's pending. Thanks for waiting.

mrchief avatar Oct 04 '22 20:10 mrchief

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Nov 18 '22 22:11 github-actions[bot]

Posting to keep this from being stale, PR is awaiting review

gandhis1 avatar Nov 18 '22 22:11 gandhis1

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 03 '23 22:01 github-actions[bot]

Not stale

gandhis1 avatar Jan 04 '23 02:01 gandhis1