Refactor TypeScript to be generic on entities.

[petejohanson]

As our team was exploring integrating redux-query into a new project using TS + React, we discovered the the built in type definitions in redux-query define the "entities state" very loosely, i.e. type Entities = { [key: string]: any } which really doesn't provide any time safety for the individual parts of the entities collection.

This PR is my first pass at making most of the types generic in T, where T is the type of "entities" defined for your particular usage.

With this PR, you might end up w/ something like:

interface Notification {
  id: string;
  name: string;
}

interface EntitiesState {
  notificationsById: { [key: string]: Notification }
}

And then your application state, would be:

interface State {
  entities: EntitiesState;
  queries: QueriesState;
}

With these changes, we can also strongly type the update, and transform props in the query config too, so those are strongly typed, including update only allowing callbacks that actually map to a property name in your EntitiesState and asserting the types parameter types for those callbacks matches, etc.

One thing I didn't do in this PR was any attempt at defaulting the generic Entities type parameter back to the original interface Entities { [key: string]: any } like it was before this PR.

Theoretically, if you do some defaulting of that type parameter, this is at least more backwards compatible. I am happy to pursue that if the maintainer(s) are interested in this general approach.

Thoughts?

[petejohanson]

Oh, and CC'ing @karevn who provided the original TS definitions recently, in case there's some reasoning behind the entities typing I've missed.

Thanks!

[karevn]

Thanks for your work! I'll take a closer look into later today.

[petejohanson]

@karevn Thanks!

In addition to the items noted in the original PR. I've pushed a few additional commits:

1. Make UpdateStrategy take a generic parameter for the body type, i.e. UpdateStrategy<Entities, Body = ResponseBody>. Doing so allows client libraries to re-use UpdateStrategy for a strongly typed mapping from Body -> Entities.

2. Update MetaValue to be any, which matches the flow typing. This matches the use in redux-query, but is theoretically a breaking change, since it makes the typing looser, and would break any consumers that are relying on the values already being typed as string.

[karevn]

Well, it's definitely a valuable improvement and seems viable. I'll need some more time to think about it - to make sure these changes will be a flexible basis for future work and won't need an another pass of changes.

[petejohanson]

@karevn Completely fair. I offered this PR as a starting point for discussion/consideration.

I do think if we wanted to merge something like this, we might want to add defaults to the new Entities generic parameter everywhere, to keep this (more?) backwards compatible. Doing so does "dirty" the code a bit, but is probably worth it.

Thanks again.

[karevn]

Even though adding strict typing to Entities is a breaking change, I think it's worth it - I doubt that the TS adoption rate is high currently, so not too many people will be get hurt. And, they'll be able to add "any" typings at their side, which is a quite straightforward change.

[rctbusk]

I will make a patch version on NPM so if people want the old typescript, they can use the previous version.

[rctbusk]

Hey @petejohanson, The code looks great. If possible, before I merge this, I would like you or your team to pursue making this backward compatible as well. Other than that, I am happy with these changes

[petejohanson]

@rctbusk Yeah, let me investigate doing that. I had just pushed a change before I saw your comment that updated the typing for connectRequest in redux-query-react that was more breaking, since it added a non-default TState generic parameter to it, e.g.:

  export type RequestConnector = <TState, TProps extends WithForceRequest = WithForceRequest>(
    mapPropsToConfigs: QueryConfigsFactory<TState>,
    options?: ConnectRequestOptions,
  ) => ConnectRequestWrapper<TProps>;

Let me switch the order of those, defaulting the TState there as well.

I will update the current PR to probably rename the generic parameters to TState, and then default them, e.g. QueryConfig<TState = EntitiesState>.

[petejohanson]

@rctbusk Ok, I've just pushed a change that adds defaulting to what I believe would be all of the places needed for backward compatibility with the generics changes.

Unfortunately, this also now includes another breaking, but seemingly needed change, to the HttpMethods type. In particular, the version in master uses a "stock" TypeScript enum, which does not seem to actually map to the right thing in QueryConfig. In particular, the options there seems to expect the method string, but normal TypeScript enums do not do that, unless they are "const enums", which unfortunately are not supported by Babel's TS support, so I would recommend not using those.

Also, doing it as an enum doesn't allow for custom HTTP methods, which is not great. I have instead changed this to be:

export type HttpMethods =
    | 'DELETE'
    | 'GET'
    | 'HEAD'
    | 'POST'
    | 'PUT'
    | 'PATCH'
    | string;

Doing so should get some nice auto-complete love in many IDEs, with suggestions for values, but also allow for other methods be allowed.

So, to summarize:

1. Generics changes, which should be backwards compatible. Since I don't have an existing redux-query + TS codebase to verify this with, it would be helpful if we could assert this somehow.
2. MetaValue changes, which is breaking, changing from string to any to allow other structures for metadata with configs.
3. HttpMethods changes - I don't see how the current implementation actually would work at runtime, since the enums would end up not passing the expected strings for the method property.

Thoughts?

[petejohanson]

Note: With the above changes dropped into our local app, combined with some work I'm doing to explore a openapi-generator generator to output typescript + redux-query, I now have a basic PoC for these type changes. Unfortunately, with the current very early stages of our work, it definitely doesn't exercise the full scope of the changes here.

@karevn I just noticed you opened #139 recently... Thoughts on the const enum versus "open" union type I have here?

[rctbusk]

@petejohanson and @karevn, I want to make sure both of your PRs are in sync with each other and make sense with each other. Want to make sure we follow best practices for Typescript and not just code for our specific needs in our products if that makes sense. I am new to typescript so I am learning here as we go and I am doing some of my own research on the side as well.

Sorry if that makes the process a little slower. Want the best for the product :)

[petejohanson]

@rctbusk No worries about this taking a little bit longer. Makes sense to get it right!

I'll let @karevn comment on the generics + MetaValue changes here, and we'll continue the HTTP methods conversation on #139, unless you would like it to all stay here.

Thanks again for the great library!

[petejohanson]

@karevn @rctbusk Ok. Rebased onto master, and added a small fix to re-add 'HEAD' as a known HTTP method.

Any other feedback/thoughts on this PR?

[rctbusk]

This looks good to me!

[petejohanson]

Thanks! When do you anticipate a release that includes this?

[rctbusk]

@petejohanson should be out now!