devvmh/redux-crud-store

Don't set collection to empty array when updating a record.

lATAl opened this issue · 17 comments

lATAl commented

Whenever updating a record, selectCollection always responds an empty array and it makes my view's empty too. What do you think if you make some changes (the record will have isUpdating or isLoading, ...) so that selectCollection will give back an array

This is a great point. I've just made a PR for this change (it's very simple - just avoid changing the record's fetchTime on the UPDATE action), but it's a breaking change so I'll merge it into 5.0.0. This is a great excuse to finally put 5.0.0 into a release though.

lATAl commented

👍

OK you've got it in 5.0.0. Let me know how it goes

lATAl commented

@devvmh After update, i has an error after fetch collection in byIdReducer @ reducers.js?8cfa:66

VM102195:1 Uncaught TypeError: payload.forEach is not a function

Could you send me the shape of your data? (EDIT: sorry, the shape of the data that comes on a FETCH_SUCCESS)

I made an unrelated change in 5.0.0 that is causing this error. It assumed that payload is an object containing at least a 'data' key. If it does NOT contain a data key, then it assumes the payload is the data, so it needs to be an array.

But I'd assume if this worked before then you either a) had your data in an array under the 'data' key or b) your data was undefined. So I must be missing something

lATAl commented

Here is response from server

{
    "success": true,
    "data": [{
        "shop_id": 10,
        "name": "Mẫu 2",
        "id": 2,
        "html": "<table border=\"1\" cellpadding=\"0\" cellspacing=\"0\" style=\"width: 100%\"><tbody><tr><td style=\"width: 206px;\">Tên Shop</td><td style=\"width: 940px;\"> </td><td> </td></tr><tr><td style=\"width: 206px;\"> </td><td style=\"width: 940px;\"> </td><td> </td></tr><tr><td style=\"width: 206px;\"> </td><td style=\"width: 940px;\"> </td><td> </td></tr></tbody></table><p> </p>",
        "default": false
    }, {
        "shop_id": 10,
        "name": "Mẫu in 1",
        "id": 1,
        "html": "<table border=\"1\" cellpadding=\"0\" cellspacing=\"0\" style=\"width: 100%\"><tbody><tr><td> </td><td> </td><td> </td></tr><tr><td> </td><td> </td><td> </td></tr><tr><td> </td><td> </td><td> </td></tr></tbody></table><p> </p>",
        "default": false
    }]
}

OK I'm super confused. src/sagas.js line 39 says:

  try {
    const response = yield call(apiClient[method], path, { params, data, fetchConfig })
    yield put({ meta, type: success, payload: response })
  } catch (error) {
    yield put({ meta, type: failure, payload: error, error: true })
  }

I'm assuming the JSON you posted above is the "response". You should see this action in your redux dev tools, with type: FETCH_SUCCESS, and response being the exact json you posted above.

The only place where payload.forEach is called in the whole codebase is src/reducers.js line 57, where it's setting

const payload = ('data' in action.payload) ? action.payload.data : action.payload

I don't see why that code wouldn't work.

Are you able to verify any of these steps?

Alternatively I could try to branch off a new version that removes this code just to see if it works for you

I'll be off the computer in 30 minutes and gone for a few hours, but I'd love to solve this today if we can.

lATAl commented

In redux dev tools, i see only success field in payload. See attach image.
screen shot 2016-08-04 at 17 20 47

Ah. I've replicated this on my computer. Give me a few minutes

lATAl commented

This is in my ApiClient, maybe it is reason?

return fetch(requestPath, {
          ...otherConfig,
          method,
          headers,
          body
        })
        .then(response => response[format]())
        .then(json => {
          if(!json.success) {
            const error = new Error();
            error.response = json;
            throw error;
          }
          return Object.prototype.toString.call( json.data ) === '[object Array]' ? json : json.data
        })

that's interesting. 5.0.0 should remove the need for checking if it's an array or a { data: [ ... ] }. But again, I've got the same problem on my machine.

I don't think I'm going to be able to solve this in the next ten hours though. Maybe I can push a workaround to a branch and you can install from Github for the moment.

ok try installing with this package.json line. I'm not really confident it'll work though...

"redux-crud-store": "uniqueway/redux-crud-store#feature/workaround",
lATAl commented

My apiClient checking type of json.data because a record response from server like this:

{
  success: true,
  data: {
    id: 1,
   ...blabla
  }
}
lATAl commented

Now, Cannot read property 'forEach' of undefined 😭

I've got it. I was mutating the action in my collectionsReducer. Fix coming in a moment

ok v5.0.1 should work. Thanks for helping out! If there are more problems, I'll be able to look at them tomorrow

lATAl commented

It works! Thanks.