mschipperheyn / normalizr-immutable

Other
123 stars 13 forks source link

Map/Record is preferred when object key is referenced #6

Closed rewmike closed 8 years ago

rewmike commented 8 years ago

When normalizing data by object key, a List is returned when a Map is better suited.

const json = {
    foo: 'foo',
    data: [
        { id: 1, name: 'Foo' },
        { id: 2, name: 'Bar' },
        { id: 3, name: 'Baz' }
    ]
};

const record = Record({
    id: null,
    name: null
});

const schema = new Schema('articles', {
    record
});

const normalized = normalize(json, {
    data: arrayOf(schema)
});

AFTER normalizr-immutable:

 {
    "entities":{ // Record
       "articles":{ // Record
             "1":{"id":1,"name":"Foo"},
             "2":{"id":2,"name":"Bar"},
             "3":{"id":3,"name":"Baz"}
       }
    },
    "result":[ // List ???
       ["foo", "foo"],
       ["data", [1, 2, 3]]
    ]
 }

AFTER normalizr:

 {
     "entities":{
         "articles":{
             "1":{"id":1,"name":"Foo"},
             "2":{"id":2,"name":"Bar"},
             "3":{"id":3,"name":"Baz"}
         }
     },
     "result":{
         "foo":"foo",
         "data":[1, 2, 3]
     }
 }
mschipperheyn commented 8 years ago

Yeah, I guess I never quite used it like this. I'll look into that. Thanks for the test!

rewmike commented 8 years ago

I changed my version to simply return results as is (which will be a Map or List). https://github.com/mschipperheyn/normalizr-immutable/blob/master/src/index.js#L248

rewmike commented 8 years ago

This change will have an impact on the schema of NormalizedRecord, but results in the expected (same as normalizr) response:

const keyedResults = normalize({
    foo: 'foo',
    data: [
        { id: 1, name: 'Foo' },
        { id: 2, name: 'Bar' },
        { id: 3, name: 'Baz' }
    ]
}, { data: arrayOf(schema) });
// {
//     "entities":{ // Record
//         "articles":{ // Record
//             "1":{ "id":1, "name":"Foo" },
//             "2":{ "id":2, "name":"Bar" },
//             "3":{ "id":3, "name":"Baz" }
//         }
//     },
//     "result":{ // Map
//         "foo":"foo",
//         "data":[1, 2, 3] // List
//     }
// }

const keyedResult = normalize({
    foo: 'foo',
    data: {
        id: 1,
        name: 'Foo'
    }
}, { data: schema });
// {
//     "entities":{
//         "articles":{
//             "1":{ "id":1, "name":"Foo" }
//         }
//     },
//     "result":{
//         "foo":"foo",
//         "data":1
//     }
// }

const rootResults = normalize([
    { id: 1, name: 'Foo' },
    { id: 2, name: 'Bar' },
    { id: 3, name: 'Baz' }
], arrayOf(schema));
// {
//     "entities":{
//         "articles":{
//             "1":{ "id":1, "name":"Foo" },
//             "2":{ "id":2, "name":"Bar" },
//             "3":{ "id":3, "name":"Baz" }
//         }
//     },
//     "result":[1, 2, 3]
// }

const rootResult = normalize({
    id: 1,
    name: 'Foo'
}, schema);
// {
//     "entities":{
//         "articles":{
//             "1":{ "id":1, "name":"Foo" }
//         }
//     },
//     "result": 1
// }
mschipperheyn commented 8 years ago

Great, I'll test it and process the changes. Prob have time this weekend or earlier to do that. Thanks

mschipperheyn commented 8 years ago

I fixed this. I still need to deploy to npm, but perhaps you can have a look before I do