Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relationships normalization #11

Closed
lvauvillier opened this issue Mar 14, 2017 · 9 comments
Closed

Relationships normalization #11

lvauvillier opened this issue Mar 14, 2017 · 9 comments

Comments

@lvauvillier
Copy link
Contributor

Hi Yury,

I'm facing new problems with the relationships normalization:

Data representation feature

With The data repesentation feature of one-to-many relationship, we lost the type "collection" if the collection contains only one element. I have some issues here with the redux-object build function. The single element collection builded is an object instead of an array containing one object.

I agree the fact data representation as a string is useful in a redux context. A naive approach can be adding a new special key to the metas to know the type of the relation (object or collection).

I'm not confident to add a special key to store it... but there is some examples to show how it can be represented:

  1. Collection with a single object
relationships: {
    comments: {
        __relation_type: 'collection',
        id: '295',
        type: 'question'
    }
}
  1. Collection with multiple objects
relationships: {
    comments: {
        __relation_type: 'collection',
        id: '295,678',
        type: 'question'
    }
}
  1. Object relationship
relationships: {
    author: {
        __relation_type: 'object',
        id: '23',
        type: 'author'
    }
}

The redux-object has also to be updated to understand these new properties.

Empty relationships

If a relationship has no data (null) or an empty collection ([]) and links, only the links will be normalized. We lost here the fact that the relation points to an empty data or collection. The redux-objectbuild will raise a lazy loading error when we want to access the relationship. Even if we know there is no data to return.

Following the precedent naive approach, i suggest to store only the collection type:

relationships: {
    author: {
        __relation_type: 'object',
    }
}

The redux-object has also to be updated to understand that the data is already loaded and not try to lazy load something.

I'm currently working on a prototype with this approach, but if you had any suggestions or direction, I'll follow it and submit a PR for the community.

@apsavin
Copy link
Contributor

apsavin commented Mar 14, 2017

Instead of additional type, we can serialize ids as [1,2,3] for collections - and check for brackets.

@lvauvillier
Copy link
Contributor Author

@apsavin with this solution, we loose the ability to compare id with strict equality.

I agree this is a more elegant solution, but the library seems to be coded with this is mind. The documentation explicit it as a feature.

Waiting for @yury-dymov suggestion :)

@lvauvillier
Copy link
Contributor Author

lvauvillier commented Mar 14, 2017

Ok understood. Serialize ids as a string containings brackets.
Yes it's an elegant solution with no breaking changes. Thanks!

The updated proposal:

  1. Collection with a single object
relationships: {
    comments: {
        id: '[295]',
        type: 'question'
    }
}
  1. Collection with multiple objects
relationships: {
    comments: {
        id: '[295,678]',
        type: 'question'
    }
}
  1. Object relationship
relationships: {
    author: {
        id: '23',
        type: 'author'
    }
}

JSON.parse() is a good candidate to do the job.

For the point 2 of empty relationships we can have:

relationships: {
    author: {
        id: '',
    }
}

Or

relationships: {
    comments: {
        id: '[]',
    }
}

@yury-dymov
Copy link
Owner

Hi @lvauvillier,

From my experience, the problem is not with json-api-normalizer but with JSON API in the first place.

My JA provider returns collection as an object if backend provides only one object. If this is different in general, please let me know. To give me a context, please attach your JSON API document, so I could figure out if json-api-normalizer can be improved or it is an issue of JSON API itself.

To resolve this issue in my project I used wrap function, where I expect to have a collection.

function wrap(object) {
  if (isArray(object) || isNull(object) || isUndefined(object)) {
     return object;
  }

  return [object];
}

@lvauvillier
Copy link
Contributor Author

lvauvillier commented Mar 14, 2017

The jsonapi spec indicates that the data field of a relationship is a resource linkage
(http://jsonapi.org/format/#document-resource-object-relationships)

Resource linkage MUST be represented as one of the following:

null for empty to-one relationships.
an empty array ([]) for empty to-many relationships.
a single resource identifier object for non-empty to-one relationships.
an array of resource identifier objects for non-empty to-many relationships.

And

A logical collection of resources MUST be represented as an array, even if it only contains one item or is empty.

If your JA provider has a different strategy, the proposal didn't break the current implementation but makes the library closer to the spec.

Also, the spec doesn't specify any constraint to have the same object type in a collection.
If we want to match exactly the spec, I think the whole shape of the relationships normalized object has to be rethought.

[edited]

New proposal, following exactly the spec (but not backward compatible):

  1. Single object
relationships: {
    author: {
        id: 23,
        type: 'author'
    }
}
  1. Single null object
relationships: {
    author: {}
}
  1. Collection
relationships: {
    comments: [{
        id: 295,
        type: 'comment'
    },
   {
        id: 296,
        type: 'comment'
    }]
}
  1. Collection with a single object
relationships: {
    comments: [{
        id: 295,
        type: 'comment'
    }]
}
  1. Empty Collection
relationships: {
    comments: []
}

@yury-dymov
Copy link
Owner

Ok, I see. Will deliver update by the end of the week for both redux-object and json-api-normalizer.

PRs are also welcome :)

@lvauvillier
Copy link
Contributor Author

lvauvillier commented Mar 14, 2017

Just submit a PR.

Finally, the normalized relationship shape is very similar to the original. I just add the case of camelizeKeys option.

I keep data and links on each relation to make redux-objects working.

  • data key available : data is already loaded
  • data key not available && linkskey available : lazyload the relation.

@yury-dymov
Copy link
Owner

Thank you! Super busy now, will review till the end on the day (PST).

@yury-dymov
Copy link
Owner

closed via PR

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

No branches or pull requests

3 participants