tests stuck in infinite loop with nested relationships, despite setting `terminateCircularRelationships` to true

[szamanr]

hi! i'm using "graphql-codegen-typescript-mock-data": "3.3.1", with setting terminateCircularRelationships: true. when i run a test using the mocks, it runs for a while and eventually runs out of memory: FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory.

if i override the relationship as an empty array, the test completes as expected.

this is my codegen config:

  ./src/@graphql/mocks.ts:
    overwrite: true
    plugins:
      - typescript-mock-data:
          addTypename: true
          terminateCircularRelationships: true
          transformUnderscore: false
          typesFile: './index.ts'

(index.ts is generated by codegen as well)

❌ this breaks:

aCycleCheckins()

✅ this works:

aCycleCheckins({
  checkIns: [],
})

my schema looks like this:

• CycleCheckins is an object which has checkIns
• • CheckIn has commits
• • • Commit has CheckIn

the loop in the generated code looks like this: aCycleCheckins: checkIns → aCheckIn: commits → aCommitConnection → aCommitEdge → aCommit: checkIn → aCheckIn

this is the generated code:

export const aCycleCheckins = (overrides?: Partial<CycleCheckins>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'CycleCheckins' } & CycleCheckins => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('CycleCheckins');
    return {
        __typename: 'CycleCheckins',
        checkIns: overrides && overrides.hasOwnProperty('checkIns') ? overrides.checkIns! : [relationshipsToOmit.has('CheckIn') ? {} as CheckIn : aCheckIn({}, relationshipsToOmit)],
        start: overrides && overrides.hasOwnProperty('start') ? overrides.start! : 'laboriosam',
        // more non-relationship fields here
    };
};

export const aCheckIn = (overrides?: Partial<CheckIn>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'checkIn' } & CheckIn => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('CheckIn');
    return {
        __typename: 'checkIn',
        commits: overrides && overrides.hasOwnProperty('commits') ? overrides.commits! : relationshipsToOmit.has('CommitConnection') ? {} as CommitConnection : aCommitConnection({}, relationshipsToOmit),
        // more fields here, but not relevant: if i comment out all relationships, test completes. if i have at least 1 relationship uncommented, if freezes.
    };
};

export const aCommitConnection = (overrides?: Partial<CommitConnection>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'commitConnection' } & CommitConnection => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('CommitConnection');
    return {
        __typename: 'commitConnection',
        edges: overrides && overrides.hasOwnProperty('edges') ? overrides.edges! : [relationshipsToOmit.has('CommitEdge') ? {} as CommitEdge : aCommitEdge({}, relationshipsToOmit)],
        pageInfo: overrides && overrides.hasOwnProperty('pageInfo') ? overrides.pageInfo! : relationshipsToOmit.has('PageInfo') ? {} as PageInfo : aPageInfo({}, relationshipsToOmit),
    };
};

export const aCommitEdge = (overrides?: Partial<CommitEdge>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'commitEdge' } & CommitEdge => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('CommitEdge');
    return {
        __typename: 'commitEdge',
        cursor: overrides && overrides.hasOwnProperty('cursor') ? overrides.cursor! : 'autem',
        node: overrides && overrides.hasOwnProperty('node') ? overrides.node! : relationshipsToOmit.has('Commit') ? {} as Commit : aCommit({}, relationshipsToOmit),
    };
};

export const aCommit = (overrides?: Partial<Commit>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'commit' } & Commit => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('Commit');
    return {
        __typename: 'commit',
        checkIn: overrides && overrides.hasOwnProperty('checkIn') ? overrides.checkIn! : relationshipsToOmit.has('CheckIn') ? {} as CheckIn : aCheckIn({}, relationshipsToOmit),
        // ...
    };
};

this is the first test where i tried using this plugin, so i don't know if it works for other models.

is it an issue in my schema that i happened to run into in the first test? or is there something wrong with the plugin and the terminateCircularRelationships option isn't working?

[lukemartin]

Having a similar issue with our usage. Would be interesting to know if there is a bug or not.

Going to look into it myself soon.

[ardeois]

That's possible we have a bug. I don't have time right now to check it, but PRs are welcome!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[szamanr]

it is still occurring which is preventing me from using this library. i am currently implementing my mock functions manually but it's a laborious workaround.

please don't close.

[emab]

I wonder if this is because the default override is an object - whereas you override to an array?

[emab]

// more fields here, but not relevant: if i comment out all relationships, test completes. if i have at least 1 relationship uncommented, if freezes.

@szamanr

Would you mind expanding on this comment a bit more? Does this happen if any relationship is present? Or it only happens when the commits relationship is uncommented?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[vikstrous2]

This is still broken based on what I've seen so far. terminateCircularRelationships is not working for us.

[rpereira-anchor]

I tested the code above and didn't reach an oom. However:

We have a big schema which is pretty much what a graph looks like. Many circular references. We're reaching an oom and by logging the creation of mocks I see the same objects being created multiple times. This is expected but can be a big problem. Depending on the number of branches this can grow quite big before a mock finds all the rules to stop creating submocks.

To have working mocks I just didn't create a new set each time a mock is created and let it reuse the same one every time.

export const aMock = (overrides?: Partial<Mock>, relationshipsToOmit: Set<string> = new Set()): { __typename: 'Mock' } & Mock => {
    relationshipsToOmit.add('Mock');
    console.log('relationshipsToOmit', relationshipsToOmit);

This made the mock generation finish without the oom.

Although this works for us I understand that beginning with a new set each time is what makes sense because you want to create a full mock with the exception of the type that created the mock in the first place. However.... OOM.

I don't know if there's a clean solution here.

The workaround we found was to have another plugin create a set of Relationships per type ahead of time to feed it to the mock creation.

As I suggestion, I would make the the relationship logic the opposite. If a Set with relationships is passed, the mocks for those relationships would be created and only at that level.

• Schemas are always growing and one can hit this problem down the road and have no idea why it happened. This happened to us.
• When creating mocks, we tend to create sub mocks manually in order to override what we want and maintain the rest of the mock auto generated so it is conformant. Might not be everyone's use case though.

If this suggestion is acceptable, I'd be open to creating a PR

[ardeois]

Interesting, we definitely have a performance/memory issue if we create a new set everytime. I'm however not sure to understand your suggestion, we're opened to PR though.

@rpereira-anchor Could you elaborate a bit more on your suggestion before getting too deep in the implementation?

Thanks !

[rpereira-anchor]

Interesting, we definitely have a performance/memory issue if we create a new set everytime. I'm however not sure to understand your suggestion, we're opened to PR though.

@rpereira-anchor Could you elaborate a bit more on your suggestion before getting too deep in the implementation?

Thanks !

@ardeois The suggestion would be a breaking change.

• instead of creating mocks for each mockable field in a mock, just create an empty object.
• if the user passes a relationship, then it would create a mock for that relationship. It would, of course, be much preferable to just be able to create a full mock. Maybe the usage of the same Set is acceptable making the full mock creation possible?

[ardeois]

I understand... I don't really like this breaking change Maybe a better fix would be to not create a new set with default values everytime new Set(_relationshipsToOmit); but instead reuse the Set. Is that what you were suggesting about reusing the Set ?

// BEFORE
export const aCycleCheckins = (overrides?: Partial<CycleCheckins>, _relationshipsToOmit: Set<string> = new Set()): { __typename: 'CycleCheckins' } & CycleCheckins => {
    const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
    relationshipsToOmit.add('CycleCheckins');
    return {
        __typename: 'CycleCheckins',
        checkIns: overrides && overrides.hasOwnProperty('checkIns') ? overrides.checkIns! : [relationshipsToOmit.has('CheckIn') ? {} as CheckIn : aCheckIn({}, relationshipsToOmit)],
        start: overrides && overrides.hasOwnProperty('start') ? overrides.start! : 'laboriosam',
        // more non-relationship fields here
    };
}

// AFTER
export const aCycleCheckins = (
    overrides?: Partial<CycleCheckins>,
    _relationshipsToOmit?: Set<string>,
): { __typename: 'CycleCheckins' } & CycleCheckins => {
    const relationshipsToOmit: Set<string> = _relationshipsToOmit || new Set();
    relationshipsToOmit.add('CycleCheckins');
    return {
        __typename: 'CycleCheckins',
        checkIns:
            overrides && overrides.hasOwnProperty('checkIns')
                ? overrides.checkIns!
                : [relationshipsToOmit.has('CheckIn') ? ({} as CheckIn) : aCheckIn({}, relationshipsToOmit)],
        start: overrides && overrides.hasOwnProperty('start') ? overrides.start! : 'laboriosam',
        // more non-relationship fields here
    };
};

Would you be able to test if that fixes your infinite loop?

[rpereira-anchor]

@ardeois That was the test I made initially. It does fix the problem.

[ardeois]

Ok so let's go with this approach then. I think a unit test will fail about this, somebody made a PR a long time ago to create a new Set everytime but I don't remember the use case Unit tests will tell us why 🙂

[rpereira-anchor]

@ardeois I tracked down the PR and PR. The behaviour was intentional. The mocks are only terminated once per branch. If we revert this, it will cause problems for that user.

[ardeois]

Thanks for tracking this out We could maybe introduce a new option to use an optimized version of terminateCircularRelationships, using your suggestion, or keep the non-optimized version to avoid breaking changes

[rpereira-anchor]

@ardeois Maybe reusing the existing option could do it. I think that making terminateCircularRelationships receive true, false or immediate could work. immediate would not recreate the set on every new mock. It would not be a breaking change as the existing usage would be safe. immediate was my first thought but anything that better conveys the intention is fine by me. WDYT?

[ardeois]

Yes very good idea !

[rpereira-anchor]

Cool! I think I'll be able to make a PR this week.

[rpereira-anchor]

@ardeois let me know of any changes you need in the PR. Thank you.