OSS Feature Decision Tree

I often get asked why a certain feature request doesn't make it into React Query. It's not an easy decision, but we have to somehow take it. Otherwise, we would just be adding more and more features to the lib that serve niche use-cases.

Before we go into detail about the decision tree I'm personally taking when deciding if a feature should make it or not, we have to be aware that not all libraries are equal. At least for the things I'm maintaining, I've found two different categories (ok, I only maintain two libraries, so it's quite possible there could be more 😅). Let's call them modular and holistic libraries (I've just made that up, not sure if it makes any sense).

Modular libraries

Modular libraries are libraries that are designed to have separate exports that are used entirely on their own. The library I'm maintaining that follows such a pattern is Remeda. Remeda is a TypeScript-first util library - think lodash meets ramda with great types.

With modular libraries, I'm not too concerned about adding more features, mainly because each export is designed to be used on it's own. As a user, in terms of bundle size, you "pay-as-you-go".

Bundlephobia says remeda is 5.3kb minzipped, but that's not the whole picture. The lib is fully treeshakable, so this only applies if you were to use every single function of remeda, which is unlikely.

As an example, if you're only using omitBy from remeda, it will only cost you 355 bytes minzipped. If you add another function, like mapToObj, it'll add another 102 bytes. So cost wise, it doesn't really matter if we add another function to the lib, as it will not affect consumers until they actually use it.

Holistic libraries

This is different for a "holistic" library like React Query. Most code lives on the queryClient, which is a class, so it's not tree-shakable.

Also, bundle size is not the only reason why a slim API surface matters more for holistic libraries. For remeda, it doesn't matter if the library exports 25 or 125 functions. You'll have to look at the docs to see what it supports anyway. Every function is conceptually its own thing that needs to be learned. They are also mostly straight-forward - you can quickly grok by their name and parameters what they're doing.

React Query on the other hand has effectively two "things" you want to do: queries (useQuery) and mutation (useMutation). You'll likely be doing them both, so you are consuming almost the whole lib every time. That means the API will get more complex over time if we add more features. Looking at useQuery, it accepts 28 options and returns an object with 24 fields. That's huge, and it might discourage newcomers because of perceived complexity.

So with every new API or feature that we introduce, we have to carefully consider if it's worth it. Here are some of the markers that I'll look at when making that decision:

How needed is it?

Is this a feature that many people request, or is it something that only one person wants? Naturally, the more people think a feature is a good idea, the more we'll think about adding it. That still doesn't mean that it's always a good idea.

One feature where I gave in to many requests is the refetchPage option, which, in hindsight, was a mistake. It's not a good API, and it introduces more problems than it solves. That's why it's so important that the features we add are in-line with our vision:

Is the feature in-line with the vision?

Basically, the question here is: Does it make sense for us to have that feature, or would this encourage anti-patterns? As an example, I've seen many requests for a callback that is triggered whenever data is read from the cache - something like onDataChanged. You could use onSuccess for that, but onSuccess is tied to a fetch happening, and if you have staleTime set, this might not be the case:

CopyonSuccess-with-staleTime: copy code to clipboard
1const useTodo = (id: number) => {
2  return useQuery({
3    queryKey: ['todos', id],
4    queryFn: () => fetchTodos(id),
5    staleTime: 5 * 60 * 1000,
6    //🚨 will not be called when fresh data is read from the cache
7    onSuccess: (data) => {
8      console.log('data changed', data)
9    },
10  })
11}

If you switch between ids and you have fresh data in the cache, the onSuccess callback won't be called, because no fetch happened. We'll just synchronously deliver data.

So while that callback might seem like a good idea at first, I'm unsure what you would do with such a callback, as it's ultimately equivalent to a useEffect:

CopyonDataChanged-with-useEffect: copy code to clipboard
1const useTodo = (id: number) => {
2  const query = useQuery({
3    queryKey: ['todos', id],
4    queryFn: () => fetchTodos(id),
5    staleTime: 5 * 60 * 1000,
6  })
7
8  React.useEffect(() => {
9    console.log('data changed', query.data)
10  }, [query.data])
11
12  return query
13}

This also means it would fire after rendering, and I haven't seen a good use-case for when this would be needed. Most cases brought forward include some form of data synchronization (putting data from useQuery into a different kind of state manager), which I would conceptually disagree with. State should ideally have one single source of truth. For async data, that is the Query Cache.

So since this idea is not in-line with how we think React Query should ideally be used, I'd be inclined not to add it to the lib. The next point will further solidify this decision:

How hard is it to implement in user-land?

Some things are straight-forward to do in user-land. The above example shows that if you really want to sync your query data reliably somewhere else, you're three lines of code away from doing that. Yes, it contains a useEffect, and you might feel a bit dirty when writing that code (as you should) but that's okay - at least it's very explicit instead of hidden behind a callback that would literally have to have the same implementation.

So if something is simple to achieve in user-land, we're more inclined to teach that pattern with documentation rather than increase the API surface for it.

Can we choose a good default value?

If we get requests for a feature where everyone expects it to work slightly differently per default, it is a good indication that this feature should rather be implemented in user-land if possible.

Does it cover all the cases?

When a feature is requested, naturally, the requester has their use-case in mind. This is totally valid - the person who opens the issue describes why they want to have it and why they think it is a good idea.

The job of the maintainer is to balance this request and compare it with other cases. As an example, I am often asked why setting suspense: true doesn't type narrow the data property:

Copysuspense: copy code to clipboard
1const { data } = useQuery({
2  queryKey: ['number'],
3  queryFn: () => Promise.resolve(5),
4  suspense: true,
5})
6
7data
8// ^? const data: number | undefined

TypeScript playground

loading states are handled by Suspense boundaries, and error states are handled by ErrorBoundaries, so why is data not just of type number ? We've had PRs contributed that tried to add that functionality with overloads.

But it's not that simple. There are a bunch of edge-cases that are not covered here. For example, you could pass enabled: false. You could also cancel the query manually while the first run of the queryFn is in-flight.

So while it's natural for an issue reporter to want their issue solved, maintainers have to look at the bigger picture. For suspense, this means we'll likely have to come up with a different API altogether.

How hard is the feature to implement and maintain?

I'm not gonna lie: Sometimes, I'm not jumping on implementing a feature because I don't know how to implement it 😅. Even if someone contributes that feature, maintainers need to fully understand it in order to maintain that feature over time.

One case where I made the "mistake" of accepting a contribution without fully understanding it is the type definitions for useQueries. It's really a beast, and I was just happy that we were getting types and had tests for them that I just shipped it.

But now, I have to reach out to the original author every time someone reports an issue with them. 🙈 This is not ideal, because sometimes, contributors lose interest once their PR has been merged. Luckily for me, Arty is still around to regularly improve the type definitions. 🙏

Let it sink in

Whatever we do, deciding in a rush is not a good idea. It takes time to find out if an API is good or not. It's also a decision that cannot be easily reverted - unless you start prefixing everything with experimental_, which honestly is something I thought about. 😁

So before you take a decision - let it sink in. Sleep over it, discuss it with friends or colleagues. Make sure you know the tradeoffs you're about to make - because there are always some.

Only then - ship it!

That's it for today. Feel free to reach out to me on twitter if you have any questions, or just leave a comment below. ⬇️

Like the monospace font in the code blocks?

Check out monolisa.dev