Skip to content

fix: handle circular references in denormalize function#44

Open
garrettg123 wants to merge 1 commit intoklis87:masterfrom
garrettg123:fix/circular-reference-clean
Open

fix: handle circular references in denormalize function#44
garrettg123 wants to merge 1 commit intoklis87:masterfrom
garrettg123:fix/circular-reference-clean

Conversation

@garrettg123
Copy link
Copy Markdown

Problem

The denormalize function causes infinite loops and stack overflow errors when processing data with circular references. This is a critical issue that can crash applications.

Example scenario:

// User creates a Project, Project references its creator
const data = {
  '@@user1': { id: 'user1', createdProjects: ['@@project1'] },
  '@@project1': { id: 'project1', createdBy: '@@user1' }
}
// Denormalizing causes: Maximum call stack size exceeded

Solution

Added circular reference detection by tracking visited entity references during traversal.

Changes (only 2 files):

  1. denormalize.ts - Added seenRefs parameter to track visited references
  2. denormalize.circular.spec.ts - Comprehensive test suite for circular scenarios

Implementation:

  • When a circular reference is detected, returns the reference string instead of recursing
  • Uses immutable Set copies for proper tracking across branches
  • Maintains backward compatibility with default parameter

Testing

Added test coverage for:

  • Simple circular references (User ↔ Project)
  • Self-referencing entities
  • Complex circular chains
  • Circular references in arrays
  • Same reference at different paths

Breaking Changes

None - the new parameter has a default value, maintaining backward compatibility.

Real-world Impact

We discovered this in production at Pulse where bidirectional relationships were causing crashes. This fix has been successfully running in our patched version.

Thank you for maintaining this excellent library! We rely heavily on normy for our React Query data management.

Prevents infinite loops when denormalizing data with circular references
by tracking seen entity references during traversal.

- Added seenRefs parameter to track visited entities
- Returns reference string when circular reference detected
- Includes comprehensive test suite for various circular scenarios

Fixes issue where entities referencing each other would cause stack overflow
@klis87
Copy link
Copy Markdown
Owner

klis87 commented Sep 14, 2025

Thank you so much for the PR. I am glad you were able to patch it yourself, but also I would like to put such fixes into the core

I will try to analyze the problem by running your tests and understanding the failure first, I might have some questions then, because usually we do not call denormalize ourselves, but it gets called automatically on mutation responses, and circular issues should be prevented by tracking queries object structures.

Anyway, I cannot wait to analyze it, but for your info, first I need to finish #1 (already finished, just need to work on documentation and improving types) - I will highly appreciate your feedback, cause I remember you use Normy for quite a while :) Generally it will allow us to have full declarative data updates, even for arrays. I also added custom array operations, so it should handle any possible case.

We discovered this in production at Pulse

I am always super happy when I hear that Normy is used in production :) I wonder, do you think we could add a section to readme like used by and put your website there? It would be very helpful to increase Normy credibility!

@klis87
Copy link
Copy Markdown
Owner

klis87 commented Oct 31, 2025

I analyzed the PR, thank you again for creating it.

However, before I can merge it, I need to understand the root cause, because you typically should not call denormalize function yourself. It is done automatically, for example imagine you have data:

const denormalizedData = { 
  id: '1',
  bestFrield: {
    id: '2',
    bestFriend: {
      id: '1',
    },
  },
}

Notice, that this is recursive, but because this is response from api, if is not recursive infinitely. The leaf node has just id key, anot not bestFriend anymore.

Then, once normalized, this object structure will be saved in usedKeys object. Then, usedKeys will be used to denormalize it, without causing infinite recursion.

Perhaps you use sth like getObjectById or getQueryFragment, then indeed it could happen. But please read https://github.com/klis87/normy/tree/master/packages/normy-react-query#getobjectbyid-and-recursive-relationships about this topic. You can pass desired object structure to avoid this problem.

If you have yet another case I did not think about, please give me the full picture, as denormalize is really internal function inside normy core and I am not sure what higher level method you actually use which leads to this issue.

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

Successfully merging this pull request may close these issues.

2 participants