Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

How to handle merge entity records into NormalizedRecord? #7

Closed
rewmike opened this issue May 11, 2016 · 5 comments
Closed

How to handle merge entity records into NormalizedRecord? #7

rewmike opened this issue May 11, 2016 · 5 comments

Comments

@rewmike
Copy link

rewmike commented May 11, 2016

NormalizedRecord cannot be merged with previous entities, records are replaced and not merged together as one would expect. Using mergeDeep (which would handle list concatenation) throws an error as entities is a Record and not a List.

import { Schema, arrayOf, normalize, NormalizedRecord } from 'normalizr-immutable';
import { Record } from 'immutable';

const record = Record({ id: null, name: null });
const schema = new Schema('articles', { record });

const results = normalize([
    { id: 1, name: 'One' },
    { id: 2, name: 'Two' },
    { id: 3, name: 'Three' }
], arrayOf(schema));

const more = normalize([
    { id: 4, name: 'Foo' },
    { id: 5, name: 'Bar' },
    { id: 6, name: 'Baz' }
], arrayOf(schema));

let state = NormalizedRecord();
console.log(state.entities === null);

state = state.merge(results);
console.log(state.entities.articles.size); // 3

// Uncaught Error: Cannot set unknown key "4" on Record
// state = state.mergeDeep(more); 

state = state.merge(more);
console.log(state.entities.articles.size); // 3 <-- Expected a size of 6 
@mschipperheyn
Copy link
Owner

Thanks. I'll take your remarks and add them to the test (feel free to do that, if you like).

@mschipperheyn
Copy link
Owner

Thinking about this a bit more, this is likely caused by the fact that the entities structure is a record and I basically abuse the Record api by creating the Record definition with the entities result. This seemed like a good idea at the time to be able to keep the ability for dot-property access in place.

This is an unwanted side effect. The most obvious workaround instead of merging is to create a new Record with the content of the original record and the new result. I suppose I could create a helper for that.

Another option would be to drop this Record approach entirely and use a map, which is the proper way of doing this, but at the expense of dot-property access.

@mschipperheyn
Copy link
Owner

I just had an idea. What if the result key would contain proxies instead of ids? In that case, you would never have to access entities directly and the entities structure could be just be based on a regular map.

Alternatively, even if we don't do that, the relation could be like so, and you could merge on the Record schema keys.

entities:{//Record with articles as a schema key
  article:{//Map with id-object relations
      3:{},
      5:{}
  }
}

If you use the proxy, this would be minimally impactful on your code. The proxy would abstract the map access away mostly. I imagine it might also be more performant.

 renderRow (rowData) {

    const { articleReducer, actions } = this.props;
    const articleObject = articleReducer.entities.articles[rowData];

    return (
        <Article
            post = {articleObject}
            {...actions} />
    );
}

render () {

    return (
        <ListView
            dataSource={ this.state.dataSource.cloneWithRows(this.props.articleReducer.result.toArray())}
            renderRow={ this.renderRow.bind(this) }
        />
    );
}

//would become

renderRow (rowData) {

    const { articleReducer, actions } = this.props;
    const articleObject = articleReducer.entities.articles.get(rowData);

    return (
        <Article
            post = {articleObject}
            {...actions} />
    );
}

render () {

    return (
        <ListView
            dataSource={ this.state.dataSource.cloneWithRows(this.props.articleReducer.result.toArray())}
            renderRow={ this.renderRow.bind(this) }
        />
    );
}

I think I will make this an option, so people can choose between the approaches. WDYT?

@mschipperheyn
Copy link
Owner

Ok, added to 0.0.3-rc branch

@rewmike
Copy link
Author

rewmike commented May 16, 2016

Though I am not currently using getState/reducerKey, that seems like a viable solution.

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

No branches or pull requests

2 participants