Possible to support object keys?

[davecoates]

Would it be possible to support objects as keys rather than forcing everything to be a string?

In my case I'm looking to do something like

const { data } = useSwr(userDetail.prepare({ id: 5 }));

which returns the same object given the same inputs. The object itself has a method on it that is called by the fetcher:

function fetcher(preparedAction) {
    return preparedAction.call();
}

As it is this doesn't work because the object returned is forced to a string here: https://github.com/zeit/swr/blob/master/src/use-swr.ts#L66

Given the cache is a Map using an object as a key works, eg. changing that line to the following appears to do what I want:

else if (!key || typeof key !== 'object') {
        // convert null to ''
        key = String(key || '');
}

Wrapping it in an array does also work:

const { data } = useSwr([userDetail.prepare({ id: 5 })]);

but the ergonomics aren't as good (especially given if you forget to wrap it in an array it just doesn't work - no errors).

Unfortunately other things assume it's a string as well - eg. CONCURRENT_PROMISES etc are just plain objects rather than a Map.

Is this something that you would be willing to extend the API to accomodate?

My alternative to make this work within the current API constraints would be to have prepare return a string that fetcher can look up into my own global cache... but I was thinking it would be nice to avoid this if swr already did that internally.

[Svish]

For simple cases, you could do something along the lines of what I did for Axios here: https://github.com/zeit/swr/tree/master/examples/axios

It basically just wraps useSWR in a new hook which uses JSON.stringify to create a key for the request object. Works fine so far here at least 🙂

[Svish]

If you need a typescript example: https://github.com/zeit/swr/tree/master/examples/axios-typescript

Although your case might be a bit simpler since that example also handles the initialData and axios response object to equal the API of useSWR

[shuding]

@davecoates I like the idea 👍

CONCURRENT_PROMISES etc are just plain objects rather than a Map.
I think we'll keep it as a plain object for now, because otherwise the keys (if you use an object as key) can't be garbage collected (string keys are fine for now).

However it's okay to wrap it with an array automatically internally:

useSwr(object) 
// equivalent to
useSwr([object])

Thanks @Svish for the JSON.stringify suggestion (it can be handy sometimes too).

[Svish]

If you wrap it in an array like that, just make sure it's actually the exact same object each time... i.e. that userDetail.prepare({ id: 5 }) doesn't return a new object each time, because if it does, it will fail the reference check and trigger a new fetch. At least pretty sure that's what will happen... make sure to check in your network log to make sure it doesn't go nuts with new requests for the same id...

[shuding]

@Svish Yeah. The ideal use case of object keys is not for "params". The ideal case should be:

const { data: user } = useSWR('/api/user', fetch)
const { data: userPhotos } = useSWR(['/api/photos', user], fetchUserPhoto)

Because SWR deep compares & caches the data (user in this example), if you're managing all the data dependencies with SWR, the issue you mentioned will never happen ;)

[Svish]

@quietshu Looks like you'd need a custom fetcher for every new data dependency type or something then? 🤔

const { data: user } = useSWR(['/api/user', userId], fetchUser)
const { data: userPhotos } = useSWR(['/api/photos', user], fetchUserPhoto)

const { data: post } = useSWR(['/api/post', postId], fetchPost)
const { data: comments } = useSWR(['/api/comments', post], fetchPostComments)

...

[shuding]

@Svish If you want to use objects as the arguments — yes. But you can hide them inside custom hooks:

const { data: user } = useUser(userId)
const { data: userPhotos } = useUserPhoto(user)

const { data: post } = usePost(postId)
const { data: comments } = usePostComments(post)

...

But under the hood, it will always be a URL + headers in the HTTP request. So no matter which pattern you want to use inside your components, I'd always suggest to transform and pass it as a string key (w/ options) to useSWR:

function useUserPhoto(user, fetcherOpts = null) {
  return useSWR(user ? ['/api/photos?uid=' + user.id, fetcherOpts] : null, fetcher)
}

In that case you don't need any custom fetcher.

[Svish]

@quietshu Don't think I like how that looks, hehe. Think I'll stick with my JSON.stringify for now. 😛

Anyways, hope @davecoates found the answer he needed! And I do agree it would be super handy if useSWR allowed to use objects as keys, although also not sure how that would work internally, so personally I'll just stick with my hack, and not complain. 🤷‍♂️😛

[shuding]

Yeah @Svish it's good to use what works the best in different cases 👍

What I was sharing is, this pattern makes great sense from the point of view of the app structure too. In complex web applications, you definitely don't want to put the APIs and fetch handlers inside every single component that uses the data. Instead you can have a /data directory with all your data sources and use them like the example above.

And that works really well for us in the past 6 months. I'll share more details in the future with a concrete example.

[Svish]

Yeah, that's pretty much what we do now, except we have an api folder with files that generates Axios request objects. Basically like this:

// src/api/foobar/index.ts
import { ApiRequest, ApiRequestDetails } from 'api';
import config from 'config';

export const foobarRequest = <Data>(
  request: ApiRequestDetails<Data>
): ApiRequest<Data> => ({
  baseURL: config('foobarBaseUrl'),
  withCredentials: true,
  ...request,
});

// src/api/foobar/user/search
import { foobarRequest } from 'api/foobar';
import { GetRequest } from 'util/useRequest';
import { User } from '.';

export type SearchResult = User;

export const getSearchRequest = (term: string): GetRequest<SearchResult[]> =>
  term
    ? foobarRequest({
        url: '/api/user/search',
        params: { term },
      })
    : null;

The Api types are just subsets of the Axios request and response types.

[Svish]

Used in search component like this:

  const { data: searchResult, isValidating } = useRequest(getSearchRequest(term));

The useRequest hook is more or less what I added in the axios-typescript example, except it uses a subset of Axios types, rather than the full ones.

[o-alexandrov]

@quietshu
cc @Svish
Please let us know, if you would accept to include the:

• useRequest
  as a built-in method of the useSWR API?

The major issue with it being an example is all of the developers, who use axios with swr and that should be the majority of those who fetch RESTful API, are all duplicating the same concept and maintain it separately from the community.
It would be better, if you would accept a refactored built-in method, that would:

• allow to initialize the hook with global settings for both axios and swr, we could name it
  • setUseSWRAxios
    • the initializer would return a named function (hook) useSWRAxios
      • personally, I'm against of naming useRequest, as the naming pattern above allows us to add other modified useSWR hooks for other libraries useSWRAnotherLib
• minimize the memory footprint, by not duplicating data in data, and response returned keys

EDIT:
As a side note, it seems @quietshu already started to name the hooks that are modifiers of useSWR as useSWRSuspense. Therefore, the naming pattern started to defer in the examples and the to-be-added API:

• [RFC] useSWRSuspense (or Suspense guards) #168

[Svish]

I suppose? Want me to try put together a PR?

[Svish]

Btw, re your second point about minimizing memory footprint, I'm pretty sure it's not really duplicating anything. The returned data is just a reference to response.data, there's no data being copied anywhere as far as I know.

[o-alexandrov]

@Svish you are not passing a reference, but creating a new object. Hence, duplicating data.
In other words, the function finishes invocation and returns the value => new object

• the values in the previously invoked function would be removed by the garbage collector

Regarding a PR, I'm also fine creating it, just want an approval from the maintainers to write the proposal.

[o-alexandrov]

@Svish
Actually, I'm wrong on the reference aspect:

[o-alexandrov]

@Svish
From the readability perspective, I'd still suggest to accept the following design, as it is clearer what's going on, separating the entities:

EDIT: reminder, the axios response includes several properties, so we introduce confusion, duplicating the reference

[Svish]

Aha, well, if you do get approval feel free to make the proposal. Just pay attention to the typings, and if at all possible, make it possible to override them. The axios types are quite "big" and allow a lot of things, which might be good to hide. For reference, here's what we actually use in our current project where I use this useRequest hook of mine, with some comments added:

// api/index.ts
import axios, { AxiosRequestConfig, AxiosResponse, AxiosError } from 'axios';

/**
 * A bare minimum subset of AxiosRequestConfig,
 * which also leaves out method forcing it to be
 * a GET request, which is the default.
 * Also note the unused <Data> type which is there
 * so the request can define its return data type.
 */
export interface ApiGetRequest<Data>
  extends Pick<
    AxiosRequestConfig,
    'baseURL' | 'headers' | 'params' | 'data' | 'withCredentials'
  > {
  url: string;
}

/**
 * Adding in the method for use in other cases than useRequest
 */
export interface ApiRequest<Data> extends ApiGetRequest<Data> {
  method?: 'DELETE' | 'POST' | 'PUT' | 'PATCH';
}

/**
 * A bare minimum subset of AxiosResponse
 */
export interface ApiResponse<Data = unknown>
  extends Pick<AxiosResponse, 'status' | 'statusText' | 'headers'> {
  data: Data;
}

/**
 * A bare minimum subset of AxiosError
 */
export interface ApiError<Data = unknown> extends Pick<AxiosError, 'message'> {
  response?: ApiResponse<Data>;
}

/**
 * Axios wrapped to accept and return the subset types
 */
export function apiRequest<Data = unknown>(
  request: ApiRequest<Data>
): Promise<ApiResponse<Data>> {
  return axios(request);
}

// util/useRequest.ts
import useSWR, { ConfigInterface, responseInterface } from 'swr';
import { apiRequest, ApiGetRequest, ApiResponse, ApiError } from 'api';

/**
 * An `ApiGetRequest`, or `null` if no request is to be performed (e.g. in the case of an empty search term)
 */
export type GetRequest<Data> = ApiGetRequest<Data> | null;

/**
 * Adjusted useSWR return type, which corrects the data and response keys
 */
interface Return<Data, Error>
  extends Omit<responseInterface<ApiResponse<Data>, ApiError<Error>>, 'data'> {
  data: Data | undefined;
  response: ApiResponse<Data> | undefined;
}

/**
 * Adjustment of the useSWR ConfigInterface to make initialData
 * work the same, i.e. so you can pass in just the data, and not
 * a full axios response, which useSWR would expect
 */
export interface Config<Data = unknown, Error = unknown>
  extends Omit<
    ConfigInterface<ApiResponse<Data>, ApiError<Error>>,
    'initialData'
  > {
  initialData?: Data;
}

export default function useRequest<Data = unknown, Error = unknown>(
  request: GetRequest<Data>,
  { initialData, ...config }: Config<Data, Error> = {}
): Return<Data, Error> {
  const { data: response, ...returned } = useSWR<
    ApiResponse<Data>,
    ApiError<Error>
  >(
    request && JSON.stringify(request),
    // @ts-ignore Typescript complains that request can be null, but this won't be called if it is
    () => apiRequest(request),
    {
      ...config,
      // If we have initial data, wrap it in a simple fake axios response
      initialData: initialData && {
        status: 200,
        statusText: 'InitialData',
        headers: {},
        data: initialData,
      },
    }
  );

  return {
    ...returned,
    data: response && response.data,
    response
  };
}

EDIT: "Flipped" the Return interface to use Omit instead of Pick and the hook to use spread operator so new things returned from useSWR would be included as well.

[Svish]

@o-alexandrov

Re the reference thing, yeah, I was pretty sure it was just a reference 😛

Re the axios response, yeah, it includes several things, which might be useful or not, but I would prefer to leave it as it is. I pull out a reference to response.data it in the return object so that the API matches useSWR (so data is the actual data, and not the full axios response), but it should still be part of the axios response. I should be able to pass the returned response from useSWRAxios to any function expecting an object of type AxiosResponse, and that type includes the data.

[davecoates]

Thanks for the suggestions everyone 👍

I think custom hooks are a good idea here for my use case

[o-alexandrov]

@Svish
To be clear, I see your solution as too opinionated, eliminating the flexibility from the user and therefore, it's unreasonable to think it'd make sense to maintain the solution that's prone to changes in axios due to the opinionated nature.

• api/index.ts you are redefining axios types for no reason, the axios request is already fully typed, you shouldn't define scenarios as developers would have different use cases

Instead of discussing your proposal, I'll just leave my unopinionated version that doesn't decide for the user how to interact with useSWR and axios.
@quietshu I ask to include it as part of the library API as devs would otherwise recreate the same thing, with axios being optionalDependency of the project:

import axios, { AxiosError, AxiosRequestConfig, AxiosResponse } from "axios"
import useSWR, { ConfigInterface } from "swr"

export const setUseSWRAxios = (
  initialRequest?: Omit<AxiosRequestConfig, "method" | "data">,
  initialConfig?: ConfigInterface,
) => {
  return function useSWRAxios<Data = unknown, Error = unknown>(
    /**
     * it's possible to add "| keyof InitialRequest or | keyof InitialConfig"
     * but we should leave the ability to override global config
     */
    request: Omit<AxiosRequestConfig, "method" | "data">,
    config?: ConfigInterface,
  ) {
    const modifiedRequest: AxiosRequestConfig = {
      ...initialRequest, // for global customizations
      ...request,
      method: `GET`,
    }
    const modifiedConfig: ConfigInterface = {
      ...initialConfig, // for global customizations
      ...config,
    }
    /**
     * Axios has a wrapper object around data => filter response
     */
    const { data: response, ...responseUseSWR } = useSWR<
      AxiosResponse<Data>,
      AxiosError<Error>
    >(
      JSON.stringify(modifiedRequest),
      async () => axios(modifiedRequest),
      modifiedConfig,
    )
    const { data, ...responseAxios } = response || {}
    return { ...responseUseSWR, responseAxios, data }
  }
}

You could then use it as follows:

const useSWRAxios = setUseSWRAxios(
  { baseURL: `https://google.com` },
  { revalidateOnFocus: false },
)

function Profile () {
  const { data, error } = useSWRAxios({ url: `/some-url` })

  if (error) return <div>failed to load</div>
  if (!data) return <div>loading...</div>
  return <div>hello {data.name}!</div>
}

[Svish]

@o-alexandrov Yeah, of course I wasn't expecting such limited types to be used in a generic hook like this. The original Axios types are fine for that, but I just wondered if it would be possible to allow for overriding the types in the setUseSWRAxios call. That way it'd work for both cases.

Anyways, comments to your proposal:

• I think initialData should be handled like in my example, otherwise the API for initialData will be different from useSWR and rather clunky. One should be able to pass something in to initialData and expect to see it returned back via data.
• I still think you shouldn't remove data from the responseAxios object because it will then no longer be a complete and valid AxiosResponse.
• Looks like the hook has no defined return type.

[shuding]

I really like the idea of setUseSWRAxios 👍

I ask to include it as part of the library API as devs would otherwise recreate the same thing, with axios being optionalDependency of the project

Here's my proposal, and I think it's more general, and works better for all libs & scenarios: #172

[nfantone]

The problem I see with this kind of custom hooks that wrap a specific fetcherFn is that it also hides away the logic for SWR keys generation - which are needed for things like mutate. I can see how they are convenient, but from an API design standpoint, that's a big hole for me. I don't see this topic being covered often. Having to JSON.stringify a request object (or any other approach) is an implementation detail. SWR keys typically also include the endpoint/URL being invoked. Hook users shouldn't know or care about that - that's why you are building the abstraction in the first place.

I'm playing with the idea of returning something like key or a partially applied mutate function from the hook itself.

import useSWR, { mutate as swrMutate } from 'swr';

function useData(endpoint /*, ... */) {
  const key = [endpoint /* , ... */];

  const mutate = (data, shouldRevalidate) => swrMutate(key, data, shouldRevalidate);

  const { data, error, revalidate, isValidating } = useSWR(key /*, ... */);

  return {
    data,
    error,
    revalidate,
    isValidating,
    mutate,
    key: [...key]
  };
}

export default useData;

You could take this idea one step further and create yet another hook for the key generation bit. That way, if you need to mutate data in some other component that is not necessarily the one that fetched the original data, you can still keep responsibilities where they belong.

// use-users.js
import useSWR, { mutate as swrMutate } from 'swr';

const isNil = value => value === null || value === undefined;

export function useKey(token /*, ... */) {
  return isNil(token) ? null : ['/users', token.trim() /*, ... */];
}

export function useMutate(token /*, ... */) {
  const key = useKey(token /*, ... */);
  return (data, shouldRevalidate) => swrMutate(key, data, shouldRevalidate);
}

function useUsers(token /*, ... */) {
  const key = useKey(token /*, ... */);
  // ...
  const { data, error, revalidate, isValidating } = useSWR(key, fetchWithToken);
  return {
    data,
    error,
    revalidate,
    isValidating
  };
}

export default useUsers;

Any thoughts on this?

[o-alexandrov]

@nfantone
Later, I found a problem with the code of @Svish and mine slightly different proposal.
When mutating the data, we should pass to the mutate function the exact same structure of the AxiosResponse

• an object, where data is a property, containing the backend response

Regarding the proposal of making the mutate function as part of the response of the useRequest (or useSWRAxios or useData in your example), swr already has an existing open issue below:

• Return a tailored mutate function with the key already applied #136

[nfantone]

@o-alexandrov Seems like #136 got implemented through #245. I didn't know about those. Good stuff!

However, the way I see it, there's still an issue with locality: the returned, partially applied, mutate function is really only available to the component using the SWR hook - everywhere else, you'd still need the original key. The problem still persists. We just moved it around.

[pke]

The docs still say one should not use objects as keys.
https://swr.vercel.app/docs/arguments#passing-objects

So whats with that now? I also see several tests that use object keys now.

[nandorojo]

For simple cases, you could do something along the lines of what I did for Axios here.

It basically just wraps useSWR in a new hook which uses JSON.stringify to create a key for the request object. Works fine so far here at least 🙂

I did something similar with swr-firestore when creating a Firestore collection query from an object here.

[shuding]

Update: I’m planning to add key serialization and object key support in #1429.

[shuding]

Added to swr@beta 🎉 ! You can now do this:

useSWR({ query: graphql`...`, variables }, request)
useSWR({ query: graphql`...`, variables }, request)
// ^recognized as the same resource

[pke]

So it does deep compare on the object fields?

[shuding]

@pke yes, same for array keys.