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

Consider adding types and logic for handling normalized state #333

Closed
markerikson opened this issue Jan 25, 2020 · 12 comments
Closed

Consider adding types and logic for handling normalized state #333

markerikson opened this issue Jan 25, 2020 · 12 comments
Milestone

Comments

@markerikson
Copy link
Collaborator

Given that we recommend normalizing state, it would be nice to have at least some amount of built-in tooling to help with that. This was also on Dan's wishlist of things to include in a Redux addon layer:

  • Containing built-in helpers for indexing, normalization, collections, and optimistic updates.

I previously asked on Twitter about the idea of adding something like this:

https://twitter.com/acemarke/status/1216766299688316928

The NgRX platform has a library called @ngrx/entity ( https://ngrx.io/guide/entity ), which contains some type definitions and helpers for working with normalized state. Per discussion with an NgRX dev, that library is already almost completely framework-agnostic - it just has a couple NgRX/Angular-specific imports.

I just opened up ngrx/platform#2333 to ask if they might be able to tweak it and make it truly agnostic, in which case we might be able to depend on @ngrx/entity ourselves.

Alternately, we could copy-paste the code with attribution.

There's also a bunch of other libraries for dealing with entity management, which I have listed at https://github.com/markerikson/redux-ecosystem-links/blob/master/entity-collection-management.md .

Key considerations:

@markerikson
Copy link
Collaborator Author

https://resthooks.io/docs/guides/redux looks interesting.

@markerikson
Copy link
Collaborator Author

Ultimately, what I'm thinking here is:

  • Fetching data from an API and putting it in Redux is a common thing to do
  • The docs describe some "standard" but complex patterns to work with:
    • async action triplets (start/success/failure)
    • dispatching those actions in a thunk
    • normalizing the state as you load it in
  • I'd like to come up with something of a 50-60% solution that minimizes the amount of code you have to write for the most common operations

@markerikson
Copy link
Collaborator Author

markerikson commented Feb 10, 2020

As an initial exploratory step, I spent some time today trying to port @ngrx/entity over to RTK.

The first step was trivial: I copied the code, and replaced the couple Angular-specific imports with createSelector from Reselect and a process.env.NODE_ENV check. (Also removed the use of deepFreeze() from the Book test fixture. This included copying over the unit test suite and fixing up the paths appropriately.

I then started reworking the "state adapter" logic to use Immer instead. This allowed me to simplify it considerably. The outer wrapping layer always made preemptive copies of state.ids and state.entities in case there were updates (which there usually were), but then did a lot of complex bookkeeping and dancing around to see if either of them had actually been mutated. This added overhead by by always copying them even if unnecessary, and then figuring out which ones had been mutated (or none).

By using Immer both at the wrapping layer and internally, I was able to delete all references to the DidMutate enum. All the tests are still passing.

I haven't actually figured out where I'm going with this process overall, or actually tried using this entity logic in any kind of an app. I'm just poking around at it to get an idea of what it does and how we might be able to use (and reuse) it.

I've pushed up my changes as a feature/entities branch if anyone wants to look at them.

@markerikson
Copy link
Collaborator Author

This evening's experimentation:

  • Swapped the argument order for stateOperator so that it's now (state, arg)
  • Added logic to check if arg is actually a Flux Standard Action, and if so, call mutator(arg.payload, state) instead
  • Verified that I could now use usersAdapter.addOne as a Redux reducer directly, by passing it to createSlice

I did run into some weird behavior with Immer, where nested produce calls didn't produce the results I expected:

immerjs/immer#533

That of course wouldn't be an issue if I hadn't just gone in and Immer-ified the guts of the entity logic.

Other than that Immer issue, I really like where this is going. Problem is, I've just forked the original @ngrx/entity API completely by swapping the arguments. As much as I'd like to stay on a shared codebase, it's starting to feel like what I want for RTK would likely be too much of a change to force on NgRx.

Current code is here: redux-toolkit/src/entities @ feature/entities.

Huh. One other note. Looks like the branch fails CI checks against TS 3.3 for some reason.

@mgcrea
Copy link

mgcrea commented Feb 12, 2020

Hey! Would be amazing to have something "officially" supported for such a common use case!

I did build something a while ago that was heavily inspired by the AngularJS 1.0 ngResource which seems to be the ancestor of @ngrx/data. Was recently thinking about leveraging RTK and maybe ky (a fetch lib) to cleanup the codebase.

At the time I tried to make things as simple as possible, so there is many things that could be improved (store layout, actual API, etc.) and except a recent rewrite to TypeScript for fun, the code is quite old!

If you wanna have a quick look: redux-rest-resource, docs.

A few things that I do personally look for:

To have a very straightforward API, eg.

import {actions as usersActions} from 'src/store/modules/users';

const UserList: FunctionComponent<Props> = () => {
  const {fetchUsers, deleteUser} = useActions(usersActions, []);
  const users = useSelector<State, User[]>(state => state.users.items);
  [...]
}
const {actions, rootReducer} = createResource({
  name: 'user',
  url: `${HOST_API_URL}/users/:_sid`,
  credentials: 'include'
});

export {actions};
export default rootReducer;

To be able to easily combine sub-resources (eg. /missions/:mission/applications/:_sid & /missions/:_sid), usually it only requires to use combineReducers on exported reducers (you need the interpolation logic for url handling though).

To be able to define custom actions:

const missionResource = createResource({
  name: 'mission',
  url: `${HOST_API_URL}/missions/:_sid`,
  credentials: 'include',
  actions: {
    publish: {method: 'POST', gerundName: 'publishing', url: './publish'},
    schedule: {method: 'POST', gerundName: 'scheduling', url: './schedule'},
    cancel: {method: 'POST', gerundName: 'cancelling', url: './cancel'},
    update: {
      assignResponse: true
    }
  }
});

Another project that you might want to check: Redux Resource that took another very different approach on API and store layout.

@ngrx/data does look pretty interesting, I might be interested to contribute / beta test something like this!

Looking forward to following your thoughts on this subject!

@markerikson
Copy link
Collaborator Author

@mgcrea : thanks for the feedback! I remember running across your lib at some point, and know I have it included in the list of entity management addons I linked up in the top comment.

I'll agree that higher-level abstractions are valuable. But, for now, I'm trying to avoid having RTK do any of the actual fetching work. That's why createAsyncThunk accepts a callback that returns Promise<YourDataResult>, and one of the reasons why I decided to try porting @ngrx/entity rather than one of the many other Redux-based "resource/entity"-type libs. That way we can focus on the parts that are truly Redux-specific, while offering the most flexibility.

I'd started to look at @ngrx/data, but one of the NgRx maintainers said it was still rather experimental and they were unsure if it would pan out (don't have the citation handy). Given that, and that @ngrx/entity is a lot simpler and the implementation was almost entirely framework-agnostic, I decided that would be a better solution for us.

@DWMiller
Copy link

@markerikson Any thoughts on adding multiple "indexes" to the normalized state created by createEntityAdapter?

For example, right now it provides data organized around ids. You can customize the selector to use whatever field you like, but it's a single field.

For comparison, I have a project where my normalization groups the code into both a "byId" and "byName" collections, and I can access it both ways.

Not sure such a thing is possible with the current setup short of duplicating the data into another slice.

@tarjei
Copy link

tarjei commented Feb 13, 2020

One problem is the query of items vs single item issue.

I.e. how do you make sure your data is not stale when you are editing one user in a list of users or you have deleted one user from your store. Often you might have used a query to have some serverside filtering before you fetched your objects - say GET /api/users?type=employees&orderBy=name&page=1&pageSize=20

Thus you rely on the ordering and membership info of that query when displaying the objects and you need a way to remember to reload or invalidate the querydata when you edit the underlying objects.

Also there is the problem of caching. I would love to be able to have a n tied system like this:

redux store -> serviceworker(SW) -> backend

Where I have good methods for invalidating the SW from both sides and where the store system handles stale while invalidate variants so I can do a query, show the user the result from the service worker, but let the serviceworker send an update to redux when the new result is ready.

One last thing, there should be a way for non CRUD requests to float through the same system - f.x. a batch update of item scores.

@markerikson
Copy link
Collaborator Author

@tarjei : that goes off into the question of cache management, which is not a problem I'm trying to solve.

The goal of this issue is simply to provide the reducer logic for doing the state updates. Where and how you make use of that reducer logic is up to you.

@DWMiller : that's an interesting idea, and worth exploring at some point. Probably not something I'm worried about for the initial release, though.

@markerikson markerikson added this to the 1.3 milestone Feb 15, 2020
@markerikson
Copy link
Collaborator Author

I've merged #352 into a new integration branch, and there's a tracking PR for that in #374 as we plan to do additional work for v1.3.0.

I've put up a v1.3.0 roadmap issue at #373 .

@markerikson
Copy link
Collaborator Author

Just released https://github.com/reduxjs/redux-toolkit/releases/tag/v1.3.0-beta.0 , which has a (hopefully) stable and documented version of createEntityAdapter.

@markerikson
Copy link
Collaborator Author

v1.3.0 with createEntityAdapter is now LIVE! https://github.com/reduxjs/redux-toolkit/releases/tag/v1.3.0

That resolves this issue.

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

No branches or pull requests

4 participants