Redux-Immutable support

[olapersson]

In order to use this in a project that uses redux-immutable some functionality would have to be overridden.

I haven't looked that deeply into it yet. But browsing the code in the v4 branch I think a pluggable merge function for the reducers and a pre-processor for _getState could work. I could then pass a function that merges immutable Maps in the reducers and calls toJS() on the state before checking for the wordpress key in connect.js.

[sdgluck]

I'm not familiar with redux-immutable so can't contribute much to this conversation. Sounds like you have something of a solid approach already in mind (a plugin?). If there's anything the library could do to make it easier, let me know, or as always, we welcome PRs. 👍

[olapersson]

Sort of a plugin should be possible (or just some simple documentation on how to implement immutable support). The reducer implementation would need to change a bit.

Taking the completeReducer as an example, it could look something like (the helper functions being part of the configuration with the plain JS functions as defaults. Haven't tested the code nor thought that much about it, just a demonstration of the idea.

// Plain JS object
function updateState(state, path, values) {
  const state = merge({}, state_)

  // LoDash set utility
  return _.set(state, path, values)
}

function mergeState(state, path, values) {
  return _.merge({}, _.get(oldState, path), newState)
}

// Immutable.js version
function updateState(state, path, values) {
  state.set(path, values)
}

function mergeState(state, path, values) {
  state.mergeIn(path, values);
}

// COMPLETE
// Place entity on the store; update query record if for component (has an id)
export function completeReducer (normalise) {
  return (state_, action) => {
    let state = state_;
    state = mergeState(
      ['entities'],
      state,
      merge(
        state.entities,
        normalise(action.data)
      )
    )

    // The action id would be null the preloadQuery method has initiated
    // the completeRequest action as they do not need a query in the store
    // (there is no component to pick it up).
    if (typeof action.id === 'number') {
      state = updateState(
        state,
        ['state', 'queries', action.id],
        {
          id: action.id,
          entities: pickEntityIds(action.data),
          paging: action.data._paging || {},
          prepared: isNode(),
          complete: true,
          OK: true
        }
      )
    }

    return state
  }
}

I've got some other things I need to finish today. But I think that in connect.js a configurable helper to get properties on the state would work.

Does this seem like a good solution to you?

[sdgluck]

Yes this seems like a fine solution to me. My main concern would be the API for hooking into the internals. Obviously the less complicated and obtrusive the better. I would prefer all the config to be required at initialisation rather than e.g. at place of component decoration - do you have thoughts on this?

[olapersson]

I agree, should be done at initialisation and shouldn't be to complicated to implement.

[olapersson]

@sdgluck Is there a working example of v4 somewhere? I'm can't seem to get the wrapped components to update.

[sdgluck]

@olapersson Not a public example, unfortunately. Working on getting a boilerplate on here soon. Are you able to provide more details? Do you mean that the component does not fetch new data on props change? Which decorator are you using?

[olapersson]

Sure, here's minimal reproduction https://github.com/olapersson/kasia-debug (you'll need to disable CORS in your browser or point the config at something that allows requests from localhost)

The request is dispatched and the store is updated, the props aren't and the shouldUpdate function is never called.

[sdgluck]

@olapersson Thank you v. much for the reproducible. 👍 I have pushed a new version to kasia@beta and amended your example to use it: https://github.com/sdgluck/kasia-debug

Let me know if that works for you (it does for me using a different wp-api endpoint), and I will merge my changes (#49) and publish.

[olapersson]

Yes, works for me.

connectWpPost seems to have a bug still, if leaving the default value for keyEntitiesBy null is returned on props.kasia.page

Pushed a repro to master here: https://github.com/olapersson/kasia-debug, keying by slug rather than id works. The request gets it right (uses slug rather than id) while _makePropsData doesn't

[sdgluck]

Thanks, there's a new version on beta that should fix that. If you could run it again it would be much appreciated. 👍

[olapersson]

Yeah, working now. :)

[sdgluck]

@olapersson Brilliant. Thanks for your help. #49 is on latest now.