Bloomca / redux-tiles

Composable way to create less verbose Redux code
https://redux-tiles.js.org/
MIT License
236 stars 10 forks source link

state result object of a request should not be null before the first request-call #13

Closed HerrVoennchen closed 7 years ago

HerrVoennchen commented 7 years ago

Hi,

watching and evaluating your library for a while and since having some fun with it. Something that handles not that great is the following:

I have a React/Redux App with redux-tiles. I bind the store properties like this:

@connect(store => {
    return {
        tasks: store.jira.myTasksRequest
    };
})
export default class TaskList extends React.Component { ...

and as long I do not call a dispatch like:

this.props.dispatch(actions.jira.myTasksRequest({}));

the store.jira.myTasksRequest property is null.

It would be much more greater, if there is the initial result object (like after a request call) there so I could bind it to some UI and logic the first place.

now I have to check the request object itself of beeing null and then may enter the result or error if th request fails.

Hope you unterstand what I mean. :)

Although great library.

Regards,

Sebastian

Bloomca commented 7 years ago

Hi, Sebastian! Thanks a lot for your interest :)

Actually, I tried as hard as possible to avoid this scenario, that it is hard to provide default value for sync tiles.

Seems you are using createSyncTile for this myTasksRequest, right (I am assuming this from your dispatching example)? If it is the case, then you can pass the initial value:

const myTasksRequestTile = createSyncTile({
  type: ['jira', 'myTasksRequest'],
  initialState: {},
});

The reason why it is null by default – because object might be not what you want by default, so null here represents absence of any data.

HerrVoennchen commented 7 years ago

Hi,

no, I use the createTile Function.

export const myTasksTile = createTile({
    type: ['jira', 'myTasksRequest'],
    fn: ({ params }) =>
        restApi.post(
            '/rest/api/2/search',
            JSON.stringify({
                jql: 'assignee = smith',
                startAt: 0,
                fields: ['id', 'key', 'summary', 'description', 'creator']
            }),
            { headers: { 'Content-Type': 'application/json;charset=UTF-8' } }
        ),
    initialState: {
        data: null,
        error: null,
        isPending: false
    }
});

As you can see, I tried to use initialState herer to accomplish this, bit it didn't work out. Property remains null.

This evening I will try to put together a complete sample application here on github so we can work it out :)

Regards

Sebastian

Bloomca commented 7 years ago

Ah, okay, I see.

Then it is a little bit more tricky – I thought about it, and for some reason I assumed that everybody will use selectors (if you would use selectors, they should return you exactly this object, which you pass as initialState).

Regarding initialState for async tiles, I am just not sure how to handle it, that's why it is unavailable. I mean, should we put under only data property, because meta is not available during normal requests, or like you are using it?

So I thought that you would use something like:

const { selectors } = createEntities(tiles, 'redux_tiles');

@connect(store => {
    return {
        tasks: selectors.jira.myTasksRequest(store),
    };
})
export default class TaskList extends React.Component { ...

So, could you please describe why you would use raw object access rather than selectors?

HerrVoennchen commented 7 years ago

Ah, querying the selector works. I used to use the @connect annotation in redux applications so maybe there is a better way or maybe use selectors directly. @connect charm is it boils the state stuff down to (mostly) plain and simple properties so the componenten knows nothing about where its data come from.

I will try to push a repo with a react/redux app without tiles so you can see my usual approach. Maybe even with some multiple requests which rely on each other (I didn't manage to accomplish that with tiles due time issues). And then we can transform the app to redux-tiles. Maybe things will get clearer (on both sides :) ). I will not exclude myself of using to much redux with redux-tiles and things can be even easier.

Bloomca commented 7 years ago

I think @connect is perfectly fine, we still abstract data, and by using selectors we also allow to change implementation later (e.g. in your example you use reducer from tiles directly, but later you might want to combine it with something else, and selector will still work).

Yeah, sure, let's go through it together and think about it! I have an example for very simple API – tiles and usage in connect.

HerrVoennchen commented 7 years ago

thank you for the links. I will go through it.

I put a sample together. Its basically that structure i use at work.

https://github.com/HerrVoennchen/react-redux-sample

I will make a branch to transform it to tiles. I'm interested in, how you would change it, feel free to fork it. :)

Edit: just pushed my version in a separate branch. Only thing I didn't get to work is to pass an axions instance as api to createMiddleware. Though I use it directly in the tile-function. Well and nesting is still a secret to me, but I'll keep digging through the docs :)

PS: if code is strange formatted, its prettier :)

Edit 2: I will next try to build a more complex request structure (like request that rely on each other) to reflect a more realistic scenario but creating tiles and using them seems already a bit easier now.

Bloomca commented 7 years ago

@HerrVoennchen I will prepare my version today in the evening, and then I'll create a PR, so we will be able to discuss it!

Bloomca commented 7 years ago

@HerrVoennchen I made a PR with my implementation, how I'd use redux-tiles there.

HerrVoennchen commented 7 years ago

Sry for the delay. Thanks for the help. I think things are clearer now and this issue is closed from my perspective. I will definitely try redux-tiles in my next bigger project which is already coming up next week.

Thanks and best regards

Sebastian

Bloomca commented 7 years ago

@HerrVoennchen sure, no problems! I've added a docs page for nesting, so hope it might clear things as well.