trackProperties method in immutableStateInvariantMiddleware.ts is not correctly managing the circularity

[paztis]

https://github.com/reduxjs/redux-toolkit/blob/master/packages/toolkit/src/immutableStateInvariantMiddleware.ts#L57

function trackProperties(
  isImmutable: IsImmutableFunc,
  ignorePaths: IgnorePaths = [],
  obj: Record<string, any>,
  path: string = '',
  checkedObjects: Set<Record<string, any>> = new Set(),
) {
  const tracked: Partial<TrackedProperty> = { value: obj }

  if (!isImmutable(obj) && !checkedObjects.has(obj)) {
    checkedObjects.add(obj)
    tracked.children = {}

    for (const key in obj) {
      const childPath = path ? path + '.' + key : key
      if (ignorePaths.length && ignorePaths.indexOf(childPath) !== -1) {
        continue
      }

      tracked.children[key] = trackProperties(
        isImmutable,
        ignorePaths,
        obj[key],
        childPath,
      )
    }
  }
  return tracked as TrackedProperty
}

recursive call to trackProperties (line 57) is not passing down the checkedObjects argument.
It means all the circularity check you do in your code is totally bypass.

That's why immutable is not working with circular references

[paztis]

I create a PR to fix this: #4844

[paztis]

Hi. No approval or reject ?
This problem is important to fix

[markerikson]

It's low on the priority list atm - all of us maintainers have been busy with other things the last few weeks. We'll try to take a look when we get a chance!

In the meantime, you could disable the default version of the immutability middleware, paste the modified version into your app, and add that to the store instead.

[soyjuanmacias]

I also have this same issue.

[paztis]

Can someone validates the PR ?
I think you can accept it easily ^^

[EskiMojo14]

I think the CI failures have been putting people off - I'll try rebasing the PR to see if the CI failures still happen.

[paztis]

Same. Are the unit tests passing without my change ?

[EskiMojo14]

yes - i rebased #4864 today and its CI is passing fine.

[markerikson]

The PR seems to break things, and I don't have the time to investigate why. It does seem to be related to the value field that's created in the test data, not the value field from the TrackedProperties type.

If someone can debug it further, figure out why this breaks things, and fix it, we can look at merging this, but it's not on my priority list at this point.

[soyjuanmacias]

@paztis I just discovered the problema and fix it.

You have complete solution in:
#4844 (comment)