Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Items as a function of inputValue #1144

Closed
bhollis opened this issue Aug 10, 2020 · 7 comments
Closed

Items as a function of inputValue #1144

bhollis opened this issue Aug 10, 2020 · 7 comments
Labels

Comments

@bhollis
Copy link

bhollis commented Aug 10, 2020

  • downshift version: 6.0.2
  • node version: v14.2.0
  • npm (or yarn) version: yarn 1.22.4

Relevant code or config/What you did/What happened/Reproduction repository:

This is a new feature request, so N/A

Problem description:

I have what I believe is a pretty common usecase for useCombobox - I'm building an autocompleter that populates the dropdown with suggestions based on what the user has typed (I'm replacing previous usage of the textcomplete library, implementing something similar to the Chrome address bar). The list of items to show in the dropdown can be a function of the inputValue - given a particular input value, I can compute what the items can be. The docs suggest that the way to do this is to set a list of items in state (useState) when the input changes via onInputValueChange and that works, but that seems a bit wrong - the state of my component is just the text in the input field, not the list of completions - those are purely a function of that input text.

I tried an approach where I maintained the inputValue state myself and used useCombobox as a controlled component, but I ran into #1108 that makes this unworkable, and it's still not as elegant as it could be.

Suggested solution:

Allow the items prop to be a function: (inputValue: string, selectionStart: number) => Item[]. This allows users to provide a function that can always compute the list of items based on the input. I'm also asking for a selectionStart argument because in my case I'm including word-by-word suggestions so if the user were to type starting in the middle of inputValue I'd want to know where they are typing so as to provide appropriate suggestions in the string. This function would be evaluated on every render (to allow for closing over other state in the function) and memoizing it is the caller's responsibility.

Happy to submit a PR for it, but I wanted to run it by you first.

@silviuaavram
Copy link
Collaborator

Why is the onInputValueChange wrong? You will use a stateful value for the items because when the value changes, you need a re-render. And you also need the items to create your item DOM elements right?

@bhollis
Copy link
Author

bhollis commented Aug 14, 2020

The state of the component is all about inputValue. The items are simply a function of that inputValue - given any inputValue I can compute the right list of items. "Thinking in React" explains why we want to include only the minimal state and compute any derived information, rather than committing derived information into state. I suspect this is rather common - when building an autocompleter the items are almost always a function of some outside environment and the inputValue, and aren't really their own state.

By forcing me to manage this derived value as state, I'm working at the wrong layer, and I'm ending up having to build out workarounds for a circular dependency - I need items to pass to useCheckbox, but I need inputValue to compute items and that comes from the return value of useCheckbox. So I end up having to do a bunch of hacks to recompute items from onInputValueChange plus anywhere else that the environment of my buildItems function changes. If I could pass a function this circular dependency wouldn't exist - useCheckbox could always determine the list of items both for its own internal use and to return for me to render, by running the function against inputValue.

You can see my rather messy WIP code here if you're interested: https://github.com/DestinyItemManager/DIM/blob/new-search-bar/src/app/search/SearchBar.tsx#L182-L227

@silviuaavram
Copy link
Collaborator

Who's useCheckbox? I don't see it in your example.

Ok, fine, it makes sense, don't keep it as a state value. Maybe I am missing something, but as far as I see, you will need some items to iterate through and render DOM elements. As long as the redux thing you're using triggers a re-render, then we shouldn't need any other stateful value.

@bhollis
Copy link
Author

bhollis commented Aug 18, 2020

Sorry, I meant useCombobox. Just a typo.

Obviously I'll need items - useCombobox can return them in the same way it can return inputValue. The problem is that useCombobox maintains inputValue as its own state, and requires items as a prop, but items should be derived from inputValue. So I'm quite stuck with a circular dependency that I can only solve by controlling inputValue, putting items in state as well, and using ugly useEffect hacks to keep it updated.

@bhollis
Copy link
Author

bhollis commented Aug 23, 2020

It's sounding like this contribution wouldn't be appreciated. I may take a look at forking Downshift for my needs - the current lifecycle of useCombobox results in unnecessary double renders and state management that could be avoided.

@bhollis bhollis closed this as completed Aug 26, 2020
@bhollis
Copy link
Author

bhollis commented Aug 26, 2020

I did manage to untangle this - I think I misunderstood how Downshift manages its internal state.

@silviuaavram
Copy link
Collaborator

@bhollis if we can improve stuff then we should. If you can give us details about how you figured it out it may help us further. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants