contentful / contentful.js

JavaScript library for Contentful's Delivery API (node & browser)
https://contentful.github.io/contentful.js
MIT License
1.19k stars 200 forks source link

Two content-objects referencing each-other produce an infinitely-deep object #171

Closed toadle closed 7 years ago

toadle commented 7 years ago

Hey guys,

so I don't know if this is intended behaviour, but it causes endless loops with for example https://github.com/paularmstrong/normalizr in a recent React-App that I'm building.

I have a simple content-model like this:

Post {
  title: string
  ...
  relatedPosts: references many Posts
}

Now when I really use this, e.g. having two Post that share a similar topic: I of course reference them to each other.

Fetching this via the Contentful Delivery API, even while giving an explicit include: 2, I get:

payload: {
  items: {
    0: {
      fields: {
        title: "article 1", 
        relatedPosts: {
          0: {
            fields: {
               title: "article 2", 
               relatedPosts: {
                 fields: {
                   title: "article 1",
                   relatedPost: [....goes on forever deep....]
                 }
               }
            } 
          }
        }
      }
    }
  }
}

When trying to work with this in a flattened store for redux this causes an endless loop. In my opinion the object should only be as deep as returned from the API - meaning stop at "article 2"'s relatedPosts.

Or is this intended?

axe312ger commented 7 years ago

I will have a look, I do not think this was intended.

axe312ger commented 7 years ago

Hey toadle,

I discussed this with a few colleagues, here is the outcome:

The include option is for the API request only, it will make sure that the payload contains all the entities of a given depth. It is not related to any feature of the JS SDK.

The automatic link resolution feature of the JS SDK which relates the content object to each other, is doing exactly what it should: It is linking references to each other using the exact same objects. There is technically no way to set a limit here. You might need to do it on your own (more see below)

We suggest to adjust your content model, since building recursive data structures may introduce even more problems. You could try the following:

If this does not help or is not acceptable, the other options are:

I hope that helps, Benedikt

toadle commented 7 years ago

@axe312ger OK, thanks for discussing it.

I followed your advice and set resolveLinks: false in my app. When using normalizr this makes sense anyway. I'll keep the relatedPosts-self reference for now and see if I get this working. Makes much more sense from a editorial-view-point. The best-matching posts are not always something that you can get via a category or similar.

To show an example, how it looks now:

normalizr-schema:

export const category = new schema.Entity('category', {

}, {
  idAttribute: value => value.sys.id,
  processStrategy: value => _.assign({}, value.fields)
});

export const post = new schema.Entity('post', {
   category: category
}, {
  idAttribute: value => value.sys.id,
  processStrategy: value => _.assign({}, value.fields)
});

post.define({
  relatedPosts: new schema.Array(post)
});

a normal reducer stays the same:

function postsReducer(state = initialState, action) {
  switch (action.type) {
    case "FETCH_POSTS_FULFILLED":
    case "FETCH_POST_FULFILLED":
      var normalizedData = normalize(action.payload.items, postListNormalizer);
      return _.assign({}, state, normalizedData.entities.post);
    default:
      return state;
  }
}

if you go for includes you can just take them directly of the includes-payload:

function categoriesReducer(state = initialState, action) {
  switch (action.type) {
    case "FETCH_POSTS_FULFILLED":
    case "FETCH_POST_FULFILLED":
      const categories = _.filter(action.payload.includes.Entry, (entry) => entry.sys.contentType.sys.id === 'category');
      const normalizedData = normalize(categories, categoryListNormalizer);
      return _.assign({}, state, normalizedData.entities.category);
    default:
      return state;
  }
}