fix(kitsu-core): fix inability to link relationship links on circular resources

[pedep]

Before this fix, this error would occasionally pop up

    TypeError: Cannot read properties of undefined (reading 'links')

      65 |   const cache = previouslyLinked[`${resource.type}#${resource.id}`]
      66 |   data[key].data = cache || link(resource, included, previouslyLinked)
    > 67 |   if (data.relationships[key].links) data[key].links = data.relationships[key].links
         |                               ^
      68 |   delete data.relationships[key]
      69 | }
      70 |

      at links (packages/kitsu-core/src/linkRelationships/index.js:67:31)
      at linkObject (packages/kitsu-core/src/linkRelationships/index.js:123:7)
      at deserialiseArray (packages/kitsu-core/src/deserialise/index.js:14:13)
      at deserialiseArray (packages/kitsu-core/src/deserialise/index.js:61:48)
      at Object.<anonymous> (packages/kitsu-core/src/deserialise/index.spec.js:795:42)

When serializing a resource with a once removed relationship to itself (A -> B -> A).
The problem seems to stem from same origin as this issue #579, where a relationships object is deleted before subsequent objects with references to it has had a change to read it

The simple solution would be to add safe navigation to the if statement, but then meta and links would be lost on any subsequent inclusions of the resource.

I was unable to reproduce this issue with array resource linkages, so only linkObject was modified. I have a feeling it might be due to something relating to pass-by-value/pass-by-reference. I wrote a spec for it, and it passed without modification, so i just removed it again. I can add it back if you think it would be relevant

I'm very open to style changes or suggestions for alternate implementations

[wopian]

I was unable to reproduce this issue with array resource linkages, so only linkObject was modified. I wrote a spec for it, and it passed without modification, so i just removed it again. I can add it back if you think it would be relevant

Even though it passed as-is, I think it's a good idea to keep the test as it's extra redundency to protect against regressions

I have a feeling it might be due to something relating to pass-by-value/pass-by-reference

linkObject will end up being called if the first call to linkRelationships enters linkArray first, so the fix in linkObject applies to both.

The code path is either:

• An object: linkRelationships > linkObject > link
• An array: linkRelationships > linkArray > (for each object in array: link > linkRelationships > linkObject > link)

[qlty-cloud-legacy]

Code Climate has analyzed commit 0d55779 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (100% is the threshold).

This pull request will bring the total coverage in the repository to 100.0% (0.0% change).

View more on Code Climate.

[pedep]

@wopian Turns out i spoke too soon regarding the array case.
I added back in the meta keys to the relationship objects after i deleted the spec. Turns out isArray does not handle meta at the top level, so i handled this in 0d55779

I'll go squash the commits in preparation for the merge when you give the OK

[wopian]

You don't need to squash the commits, the PR merge itself squashes them 👍

Turns out isArray does not handle meta at the top level, so i handled this in 0d55779

Good spot! D:

[wopian]

Thank you! 🎉

[wopian]

Released in 10.0.0-alpha.26