Skip to content

Experimenting with normalizr#350

Closed
machour wants to merge 13 commits intogitpoint:masterfrom
machour:redux-normalizr
Closed

Experimenting with normalizr#350
machour wants to merge 13 commits intogitpoint:masterfrom
machour:redux-normalizr

Conversation

@machour
Copy link
Copy Markdown
Member

@machour machour commented Sep 23, 2017

This is not a real PR, but more a first experiment conducted using Redux with Normalizr, and some of my findings. Maybe this could lead to good ideas for the big refactoring due on our side to better handle the state.

As a reminder, our current Store problem is that we only have a user / repo in the state at runtime, and if you open two different profiles on two different screens, last data fetched is shown on both.

I came across https://github.com/reactjs/redux/tree/master/examples/real-world a few days ago, and decided to try to implement something similar in GitPoint.

The main idea is to have a Redux middleware taking care of most of the APIs calls, and normalizing the stored data into entities and paginations.

Here's what is working and worth looking at in this PR:

  • Normalization of User/Schema/Event
  • Pagination of the list of followers and repositories for a user
  • Minimizing the store footprint by excluding some predictable fields from the entity

I'm going to comment some files changes in the diff to give some more insight.

Comment thread src/api/api.client.js
@@ -0,0 +1,482 @@
import { normalize, schema } from 'normalizr';
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the old api/index.js file, with some additions in the end : The client api.middleware uses to query the REST api and paginate the results

Comment thread src/api/api.schema.js
import { eventSchema } from '../event/event.schema';

// Schemas for Github API responses.
export const Schemas = {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Schemas are used by Normalizr to understand the JSON structure

Comment thread src/api/api.type.js
@@ -0,0 +1,10 @@
export * from '../event/event.type';
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making all types available in this file.

onPress={() => this.navigateToRepository(userEvent, true)}
>
{userEvent.payload.forkee.full_name}
{userEvent.payload.forkee}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After normalization, userEvent.payload.forkee is simply the repo name (which can be later found in entities.repo)

underlayColor={colors.greyLight}
onPress={() => navigation.navigate('Repository', { repository })}
onPress={() =>
navigation.navigate('Repository', { name: repository.full_name })}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of our screens can be passed either a "string" or an "object" for the principal entity they're dealing with.
For example, user-profile can either take a login name, or a user object. This is really confusing and generate a lot of checks.

By switching to normalizer, we can now pass only the login, and let the middleware fetch the data from store or api :)

Comment thread src/event/event.schema.js
forkee: repoSchema,
member: userSchema,
},
});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of my favorite files :)
We describe the event structure and link nested objects to their own schema

showFullName
/>}
onEndReachedThreshold={0.5}
onEndReached={() => getFollowers(login, true)}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is how pagination is done :) Passing true as the second parameter for the data fetcher


const orgs = [];

if (!user) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing this, we could draw the screen, display the information we already have (user avatar and name) and draw loading elements for the rest until the data is retrieved from the API.


const { entities: { repos }, pagination: { reposByUser } } = state;
const reposPagination = reposByUser[login] || { ids: [] };
const repositories = reposPagination.ids.map(id => repos[id]);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's how we reconstruct the repositories array, we fetch the ids from the pagination object and map them back to entities.repos

Comment thread src/user/user.schema.js
{
idAttribute: user => user.login.toLowerCase(),
processStrategy: entity =>
omit(entity, [
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this fields can be guessed by having just the username. We remove them from the object to minimize our footprint

@machour
Copy link
Copy Markdown
Member Author

machour commented Sep 23, 2017

Finally, here's how to test this PR :

  • Load the App
  • Click on a user name in the events screen
  • Click on either his repos or followers
  • From there, click either on a repo or a username

This is the only testable path so far.

Here's how the redux store looks like during that :

  • Events loaded, here's what we have on our users:

screen shot 2017-09-23 at 5 19 17 pm

  • User clicked, profile page loaded:

screen shot 2017-09-23 at 5 19 32 pm

  • Number of repositories clicked :

screen shot 2017-09-23 at 5 20 15 pm

  • Clicking on the samdark/laradock repo:

screen shot 2017-09-23 at 5 27 52 pm

'releases_url',
'deployments_url',
]),
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now this
screen shot 2017-09-23 at 5 27 35 pm
Looks definitely better than this:
screen shot 2017-09-23 at 5 23 14 pm

@machour
Copy link
Copy Markdown
Member Author

machour commented Oct 9, 2017

Deprecated by #457

@machour machour closed this Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant